Use Gradle Worker API for R8 minimization - #2172
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates R8-based minimization from direct ExecOperations.javaexec invocation to the Gradle Worker API, aiming to improve isolation and align with Gradle’s preferred execution model for heavy tools.
Changes:
- Inject
WorkerExecutorintoShadowJarand route R8 minimization through it. - Replace
ExecOperations.javaexecR8 execution with aWorkAction-based implementation. - Document the change in the Unreleased changelog and update the Kotlin ABI dump to reflect the new protected accessor.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.kt | Injects WorkerExecutor, deprecates ExecOperations, and passes the executor into R8 minimization. |
| src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/R8Minimizer.kt | Reworks R8 invocation to run via a Gradle worker and introduces R8WorkAction. |
| docs/changes/README.md | Adds an Unreleased entry describing the Worker API migration for R8 minimization. |
| api/shadow.api | Updates ABI dump to include the new protected getWorkerExecutor() accessor. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ca8c58e to
69a5f69
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/MinimizeTest.kt:701
- Using
single { ... }makes this functional test flaky if Gradle logs multiple worker daemon startups (or duplicates the line). Prefer selecting the first matching line instead.
val workerCommand =
result.output.lineSequence().single { "Starting process 'Gradle Worker Daemon" in it }
assertThat(workerCommand).contains("-Xmx640m")
src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/R8Spec.kt:21
- KDoc says the value uses JVM memory notation (e.g.
512m), but the default value is set fromRuntime.getRuntime().maxMemory().toString()(a raw byte count). Clarify in the documentation that a plain byte count is also accepted (or adjust the default to match the documented notation).
* Defaults to the effective maximum heap size of the Gradle daemon. The value uses JVM memory
* notation, such as `2g` or `512m`.
src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/MinimizeTest.kt:679
- Using
single { ... }makes this functional test flaky if Gradle logs multiple worker daemon startups (or duplicates the line). Prefer selecting the first matching line instead.
This issue also appears on line 699 of the same file.
val workerCommand =
result.output.lineSequence().single { "Starting process 'Gradle Worker Daemon" in it }
assertThat(workerCommand).contains("-Xmx${768L * 1024 * 1024}")
c836572 to
9e4f32e
Compare
Refs: - https://docs.gradle.org/current/userguide/worker_api.html - https://github.com/Goooler/agp-sources/blob/d383e6c2d704f6f93397fceb9a424e0082dfa164/9.3.1/com.android.tools.build/gradle/com/android/build/gradle/internal/tasks/R8Task.kt#L327-L337 - https://github.com/Goooler/agp-sources/blob/d383e6c2d704f6f93397fceb9a424e0082dfa164/9.3.1/com.android.tools.build/gradle/com/android/build/gradle/internal/tasks/R8Task.kt#L523-L579
50f83a3 to
c77e1c8
Compare
|
Based on Goooler/ktfmt@4d98e5b: Summary Statistics
|
|
No need to land this for now. |
Refs: