Skip to content

Use Gradle Worker API for R8 minimization - #2172

Open
Goooler wants to merge 1 commit into
mainfrom
g/20260810/worker-api-for-r8
Open

Use Gradle Worker API for R8 minimization#2172
Goooler wants to merge 1 commit into
mainfrom
g/20260810/worker-api-for-r8

Conversation

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.

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 WorkerExecutor into ShadowJar and route R8 minimization through it.
  • Replace ExecOperations.javaexec R8 execution with a WorkAction-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.

@Goooler
Goooler force-pushed the g/20260810/worker-api-for-r8 branch 2 times, most recently from ca8c58e to 69a5f69 Compare August 10, 2026 04:29
@Goooler
Goooler requested a lite review from Copilot August 10, 2026 04:37

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.

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 from Runtime.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}")

@Goooler
Goooler force-pushed the g/20260810/worker-api-for-r8 branch from 69a5f69 to c2ae918 Compare August 10, 2026 04:53
@Goooler
Goooler force-pushed the g/20260810/worker-api-for-r8 branch from c2ae918 to c836572 Compare August 10, 2026 05:00
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