[Goooler]

Benchmark Environment & Configuration

• Project: ktfmt
• Task: :ktfmt:shadowJar (enableAutoRelocation = true)
• Gradle Version: 9.6.1
• Profiler Setup: Gradle Profiler (2 warm-ups, 5 measured iterations)
• Build Cache: Enabled for upstream compilation tasks (compileKotlin hits cache; only shadowJar packaging and bytecode remapping are measured)

---

Performance Comparison

Iteration	df3998f (Sequential Baseline)	ab94ae9 (Parallel Optimization)	Difference
Warm-up 1 (Cold)	103,971.88 ms (~103.97s)	31,404.49 ms (~31.40s)	-69.80%
Warm-up 2 (Warm)	42,167.22 ms (~42.17s)	9,186.79 ms (~9.19s)	-78.21%
Measured 1	41,782.65 ms (~41.78s)	8,870.32 ms (~8.87s)	-78.77%
Measured 2	41,601.39 ms (~41.60s)	8,946.68 ms (~8.95s)	-78.49%
Measured 3	41,775.68 ms (~41.78s)	9,280.09 ms (~9.28s)	-77.79%
Measured 4	41,728.49 ms (~41.73s)	8,914.80 ms (~8.91s)	-78.64%
Measured 5	41,819.90 ms (~41.82s)	8,880.44 ms (~8.88s)	-78.76%
Mean (Measured)	41,741.62 ms (41.74s)	8,978.47 ms (8.98s)	-78.49%
Median (Measured)	41,775.68 ms (41.78s)	8,914.80 ms (8.91s)	-78.66%

---

Summary

• Speedup: ~4.65x faster.
• Time Reduction: The average execution time of :ktfmt:shadowJar dropped from ~41.7s down to ~8.9s (a ~78.5% decrease in build time) when relocating large dependencies like kotlin-compiler-embeddable.

---

• CHANGELOG's "Unreleased" section has been updated, if applicable.

[Copilot]

🟡 Changes recommended

Address cancellation handling, relocator thread safety, and non-class resource buffering.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request parallelizes bytecode remapping while preserving ZIP entry order.

Changes:

• Adds coroutine-based remapping and serialized ZIP writing.
• Refactors remapping and file reading to use byte arrays.
• Adds functional tests, dependency configuration, and a changelog entry.

Review findings:

• Critical, 3 votes: Cancellation can leave channel sends blocked after remapping failure.
• Moderate, 2 votes: User-provided relocators may not be thread-safe when invoked concurrently.
• Critical, 3 votes: Buffering complete non-class resources can cause memory spikes or OOM.

File summaries

File	Description
src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt	Parallel remapping and queued ZIP writes
src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/RelocatorRemapper.kt	Byte-array remapping support
src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/GradleCompat.kt	File-reading helper
src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/ParallelRelocationTest.kt	Parallelism, determinism, and failure tests
gradle/libs.versions.toml	Coroutines dependency version
CHANGELOG.md	Unreleased change entry
build.gradle.kts	Compile-only coroutine dependency

Review details

• Files reviewed: 6/7 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Unresolved findings concern unnecessary no-relocation coroutine work and unreliable or insufficient parallelization test coverage.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt:220

• This invokes runBlocking for every direct resource even when there are no pending remapped classes (for example before the first class or whenever relocators are disabled). Guard the flush with pendingEntries.isNotEmpty() so the normal resource path does not pay coroutine setup overhead.

          runBlocking { flush() }

src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyActionTest.kt:160

• This benchmark only launches coroutines around ByteArray.remapClass; it never invokes ShadowCopyAction or its visitFile scheduling path. It can therefore pass even if the production copy action remains sequential, so the PR's core parallelization is not covered. Exercise the action with a deterministic instrumented relocator/barrier instead.

    fun remapParallel() = runBlocking {
      classes
        .map { (path, bytes) ->
          async(Dispatchers.Default) {
            bytes.remapClass(relocators = relocators, path = path)

• Files reviewed: 8/9 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

ShadowCopyAction.kt contains a compilation error and an unresolved remap-failure propagation issue.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt:202

• Each async is a child of the same runBlocking job. If one remap fails, that child cancels the parent and the other pending remaps before flush() reaches them; when the stream then encounters another entry, the cancellation can be caught and reported as an error for that unrelated entry instead of reliably propagating the original Error in ASM processing class <path>. Run these children under a supervisor scope (while still awaiting them in order), or explicitly retain and propagate the first remap failure.

            val deferred =
              scope.async(Dispatchers.Default) {
                rawBytes.remapClass(relocators = relocators, path = path)
              }

• Files reviewed: 8/9 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review details

Suppressed comments (1)

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt:286

• The sliding window limits only the number of entries, not the amount of data retained: each pending task captures its full input rawBytes, and completed Deferreds retain their remapped ByteArray until they are written. A JAR containing a few unusually large class files can therefore create a large memory spike despite the comment claiming that this bounds memory consumption. Bound the window by total queued byte size (or otherwise account for class-byte memory), not only by entry count.

    private const val MAX_PENDING_ENTRIES = 128

• Files reviewed: 8/9 changed files
• Comments generated: 0 new
• Review effort level: Lite