Skip to content

Parallelize bytecode remapping in ShadowCopyAction - #2302

Open
Goooler wants to merge 9 commits into
mainfrom
g/20260912/parallel
Open

Parallelize bytecode remapping in ShadowCopyAction#2302
Goooler wants to merge 9 commits into
mainfrom
g/20260912/parallel

Conversation

@Goooler

@Goooler Goooler commented Sep 12, 2026

Copy link
Copy Markdown
Member

No description provided.

@Goooler
Goooler force-pushed the g/20260912/parallel branch from f5a4186 to 6ce4373 Compare September 12, 2026 15:47
@Goooler
Goooler force-pushed the g/20260912/parallel branch 2 times, most recently from e47542a to de331c0 Compare September 13, 2026 04:56
@Goooler
Goooler marked this pull request as ready for review September 13, 2026 04:56
@Goooler
Goooler requested a lite review from Copilot September 13, 2026 04:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Comment on lines +246 to +250
val rawBytes = fileDetails.readBytes()
sendEntry(
entryName = relocated,
fileDetails = fileDetails,
deferredBytes = CompletableDeferred(rawBytes),
Comment on lines +261 to +262
runBlocking {
channel.send(
Comment on lines +232 to +234
val deferred =
scope.async(Dispatchers.Default) {
rawBytes.remapClass(relocators = relocators, path = path)
@Goooler
Goooler force-pushed the g/20260912/parallel branch from de331c0 to 7d8fe17 Compare September 13, 2026 08:14
# Conflicts:
#	src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt
@Goooler
Goooler force-pushed the g/20260912/parallel branch from 3dc4f9e to bd7b680 Compare September 13, 2026 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants