Skip to content

Optimize Maven configuration warm path - #1182

Merged
brunoborges merged 7 commits into
actions:mainfrom
brunoborges:brunoborges-optimize-maven-configuration-on-the-warm
Jul 30, 2026
Merged

Optimize Maven configuration warm path#1182
brunoborges merged 7 commits into
actions:mainfrom
brunoborges:brunoborges-optimize-maven-configuration-on-the-warm

Conversation

@brunoborges

@brunoborges brunoborges commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Description:
Warm JDK setup runs still paid Maven XML initialization cost even when the runner only needed the default new-file configuration path. This change avoids eager Maven XML work by using deterministic serializers for default settings.xml and new toolchains.xml, deferring Maven configuration modules until after Java setup, and using fast-xml-parser only when an existing toolchains.xml must be parsed for merge.

The Maven settings and toolchain work now runs concurrently where files are independent, while toolchain updates remain serialized so multi-version runs keep deterministic entries. The remaining xmlbuilder2 dependency was removed from runtime and tests, which drops the generated dist/setup/697.index.js DOM/XML-builder chunk that CodeQL flagged. The merge path now preserves custom root attributes, custom toolchains, nested attributes/text, partial JDK entries without IDs, duplicate filtering, and XML escaping through targeted regression tests.

Benchmark workflow assets were removed from this repository; Maven warm-path benchmarking moved to actions/setup-java-benchmarks in actions/setup-java-benchmarks#9.

Related issue:
Fixes: #1181

Check list:

  • Ran npm run check locally (format, lint, build, test) and all checks pass.
  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

Avoid eager Maven XML initialization on warm JDK runs by using deterministic serializers for new Maven settings/toolchains files, lazy-loading xmlbuilder2 for existing toolchains merges, and deferring Maven configuration modules until after Java setup.

Add targeted tests for XML escaping, lazy xmlbuilder2 loading, concurrent Maven configuration, and a manual benchmark workflow for warm-path validation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4dc58426-5e20-44cb-af16-8da0965fac3b
@brunoborges
brunoborges requested a review from a team as a code owner July 30, 2026 16:23
Copilot AI review requested due to automatic review settings July 30, 2026 16:23

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

Note

Copilot was unable to run its full agentic suite in this review.

This PR refactors Maven-related configuration to reduce startup cost (especially around XML handling) and improve orchestration by parallelizing independent steps.

Changes:

  • Added lightweight XML escaping helpers and switched Maven settings/toolchains XML generation to avoid eagerly loading xmlbuilder2.
  • Refactored toolchain ID validation into a dedicated module and updated setup flow to overlap Maven auth + toolchains configuration.
  • Added targeted tests and a benchmarking workflow/script to measure Maven configuration performance and bundle size impact.
Show a summary per file
File Description
src/xml.ts Introduces reusable XML escaping helpers for text/attributes.
src/toolchains.ts Defers xmlbuilder2 loading to merge path only; adds manual XML generation for new toolchains and reorganizes toolchain ID validation export.
src/toolchain-ids.ts Extracts validateToolchainIds into a dedicated module.
src/setup-java.ts Reworks orchestration to collect toolchain configs during install and configure Maven (auth/toolchains/args) with more concurrency.
src/auth.ts Replaces xmlbuilder2 settings.xml generation with manual XML + escaping.
dist/setup/874.index.js Bundled output updates reflecting new lazy-loading / module splits.
dist/setup/81.index.js New bundle chunk for auth/xml/gpg changes.
dist/setup/767.index.js Bundle module id remapping for dependencies.
dist/setup/463.index.js Bundled output updates reflecting new lazy-loading / module splits.
dist/setup/451.index.js New bundle chunk for toolchains/xml/toolchain-ids changes.
dist/setup/242.index.js Bundle module id remapping for node externals.
dist/setup/220.index.js Bundled output updates reflecting new lazy-loading / module splits.
dist/setup/172.index.js New bundle chunk for Maven args logic.
tests/toolchains.test.ts Updates tests for async toolchain generation and adds escaping coverage.
tests/setup-java.test.ts Updates orchestration tests and adds a concurrency/overlap test for Maven configuration.
tests/maven-xml-loading.test.ts Adds coverage ensuring xmlbuilder2 is only loaded on merge path.
tests/benchmark-maven-configuration.sh Adds local benchmark harness for timing and bundle sizing.
tests/auth.test.ts Adds escaping semantics test for settings.xml generation.
.github/workflows/benchmark-maven-configuration.yml Adds workflow to run benchmarks across OS/cache/version/toolchains matrices.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 11/21 changed files
  • Comments generated: 3
  • Review effort level: Low

Comment thread src/toolchains.ts
Comment thread src/setup-java.ts Outdated
Comment thread __tests__/toolchains.test.ts Outdated
Make the toolchain XML generator consistently async, remove redundant Maven configuration await handling, and reuse the existing XML test helper.

Configure CodeQL to skip generated dist output so newly split vendored chunks do not report duplicate generated-code alerts.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4dc58426-5e20-44cb-af16-8da0965fac3b
@brunoborges

Copy link
Copy Markdown
Contributor Author

Addressed the Copilot review feedback in 7eadc3b:\n\n- Made generateToolchainDefinition consistently async and updated tests to await both fast and merge paths.\n- Removed the redundant second await of mavenConfiguration in setup-java.ts.\n- Removed the duplicate nested xmlElementText helper and reused the file-level helper.\n- Added CodeQL config for the reusable workflow so generated dist/** files are excluded from analysis; the failing alerts were in the newly split generated xmlbuilder2 bundle chunk.

Document XML attribute escaping, simplify Maven configuration awaiting, and add a regression test that feeds fast-path toolchains output into the merge path.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4dc58426-5e20-44cb-af16-8da0965fac3b
Use fast-xml-parser for existing toolchains.xml parsing and serialize merged Maven toolchains deterministically. This removes the bundled xmlbuilder2 DOM/XML builder chunk from dist while preserving merge behavior for custom attributes, custom toolchains, partial entries, duplicate filtering, and escaping.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4dc58426-5e20-44cb-af16-8da0965fac3b
Remove the Maven warm-path benchmark workflow and helper script from setup-java. Benchmark coverage is being moved to actions/setup-java-benchmarks so this action repository only carries the runtime optimization, tests, and generated distribution artifacts.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4dc58426-5e20-44cb-af16-8da0965fac3b
@brunoborges
brunoborges merged commit 5827477 into actions:main Jul 30, 2026
87 checks passed
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.

Optimize Maven configuration on the warm JDK path

2 participants