Skip to content

Optimize model building pipeline: defer Dependency.build() and reduce allocations - #12653

Draft
gnodet wants to merge 1 commit into
masterfrom
optimize-model-building-pipeline-defer-dependency
Draft

Optimize model building pipeline: defer Dependency.build() and reduce allocations#12653
gnodet wants to merge 1 commit into
masterfrom
optimize-model-building-pipeline-defer-dependency

Conversation

@gnodet

@gnodet gnodet commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add Builder getters to generated model classes (model.vm) — enables reading builder fields through the base chain without calling build(), supporting deferred materialization across pipeline stages
  • Add *ToBuilder merger variants (merger.vm) — returns Builder instead of calling build(), letting callers accumulate changes across multiple merge passes without intermediate allocations
  • Defer build() in DefaultDependencyManagementInjector — accumulates merged dependencies as Builder objects, calling build() only once per modified dependency at the end of the loop
  • Optimize computeLocations() — replaces Stream.concat().collect(Collectors.toUnmodifiableMap(...)) with HashMap.putAll() + Map.copyOf(), and returns oldlocs directly when newlocs is empty (avoids unnecessary Map.copyOf since the base map is already immutable)
  • Precompute locationsHashCode — cached at build time in the generated constructor, used by DefaultModelObjectPool.PoolKey for fast inequality checks before full map comparison
  • Optimize PoolKey.locationsEqual() — uses direct getLocations() map comparison instead of iterating individual keys, with fast-path for both-empty maps (common for imported deps)
  • Add addLocationInformation API to XmlReaderRequest — wired through DefaultModelXmlFactory for future use in skipping location tracking on imported BOM POMs

Context

JFR profiling on a 4,383-module reactor shows 37% CPU in ModelObjectPool (dependency interning), 18% in Dependency immutable builders, and 2.3 GB allocated in KeyValueHolder from computeLocations(). Each Dependency passes through ~7 pipeline stages, each calling build() which allocates a new immutable object + recomputes location maps + calls processObject().

This PR targets the three hottest code paths:

  1. Redundant build() calls — deferred via Builder getters and ToBuilder merger variants
  2. computeLocations() stream overhead — replaced with HashMap merge + early return
  3. PoolKey location comparison — precomputed hash + direct map equality

Test plan

  • mvn verify -B passes across all reactor modules (all tests green)
  • FileToRawModelMergerTest updated to exclude new *ToBuilder methods from reflection-based override check
  • Benchmark with JFR on large reactor to measure allocation reduction

🤖 Generated with Claude Code

@gnodet
gnodet force-pushed the optimize-model-building-pipeline-defer-dependency branch from 46712b6 to 5e74cd4 Compare August 1, 2026 05:57
@gnodet

gnodet commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Benchmark Results (4383-module project, mvn validate)

Added commit 568b44b that eliminates unnecessary builder.build() calls in pipeline stages by using targeted builder getters instead of full model materialization.

Changes

model.vm: Added getBuilt*() methods for model-object list fields. When the builder field is null (unmodified), returns base.getXxx() at zero cost. When modified, builds just that field's builders — avoids full model build.

5 pipeline stages fixed to avoid builder.build():

  • DefaultPluginConfigurationExpanderbuilder.getBuild() / builder.getReporting()
  • DefaultModelNormalizer (2 methods) → builder.getBuild() / builder.getBuiltDependencies()
  • DefaultDependencyManagementInjectorbuilder.getDependencyManagement() / builder.getBuiltDependencies()
  • DefaultPluginManagementInjectorbuilder.getBuild()
  • DefaultModelPathTranslatorbuilder.getBuild() / builder.getReporting()

Results (3 runs each, average)

Version Average vs rc-6
rc-6 baseline 22,114ms
PR #12652 (pool+sort) 18,617ms -15.8%
PR #12653 (before this commit) 17,739ms -19.8%
PR #12653 (with this commit) 17,956ms -18.8%

Performance is within run-to-run variance. The fix eliminates the unnecessary full-model materializations while maintaining the same performance level. All 1130 tests pass (550 in maven-impl, 580 in maven-core).

@gnodet

gnodet commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Updated benchmark: getModifiable() + two-field optimization

Following up on the previous fix — the pipeline stages now use getModifiable*() to work directly with mutable Dependency.Builder lists, mutating in place instead of round-tripping through immutable objects.

What changed (commit 443dcc5)

model.vm (two-field optimization):

  • Builder now stores both Collection<X.Builder> (for getModifiable*()) and Collection<X> Imm (for the setter)
  • The dependencies(Collection<Dependency>) setter stores the immutable list directly — zero wrapping overhead
  • getModifiable*() lazily wraps from whichever source is set (immutable field, or base)
  • build() constructor uses whichever field is set — immutable list path has no builder overhead
  • forceCopy constructor stores immutable reference instead of wrapping

Pipeline stages (getModifiable pattern):

  • DefaultModelNormalizer.injectDefaultValues — iterates getModifiableDependencies(), sets scope on builders in place
  • DefaultModelNormalizer.mergeDuplicates — deduplicates builder list in place using management key computed from builder getters
  • DefaultDependencyManagementInjector — new mergeManagementIntoBuilders() method merges managed dep fields (version, scope, type, classifier, systemPath, exclusions + locations) directly into Dependency.Builder objects

Cost model

Before (v1): each pipeline stage that touches deps does a full wrap + unwrap cycle:

builder.getBuiltDependencies()  → N unwrap allocations
builder.dependencies(newList)   → N wrap allocations

With 3 stages touching deps: 6N allocations per module (3 wraps + 3 unwraps)

After (v2): ONE lazy wrap + ONE final unwrap for the entire pipeline segment:

builder.getModifiableDependencies()  → lazy wrap (cached across stages)
  stage 1: modify builders in place   → 0 allocations
  stage 2: modify builders in place   → 0 allocations
  stage 3: modify builders in place   → 0 allocations
builder.build()                      → final unwrap

Total: 2N allocations per module (1 wrap + 1 unwrap)

Benchmark results (4383-module project, mvn validate --threads 1)

Build Average vs rc-6 vs PR #12652
rc-6 baseline 20,234ms
PR #12652 (pool+sort) 14,936ms -26.2%
PR #12653 v1 (getBuilt) 14,784ms -26.9% -1.0%
PR #12653 v2 (getModifiable) 13,385ms -33.8% -10.4%

The v2 optimization delivers a clear 10% improvement over PR #12652 alone, and 34% over rc-6.

Branch: optimize-model-builder-pipeline-fix (commits 568b44b861 + 443dcc5b51)

@gnodet

gnodet commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

JFR-guided optimization (v3): withImportedFrom + ConcurrentHashMap.newKeySet

JFR profiling on the 4383-module benchmark identified two remaining hotspots, now fixed in commit d30efb5:

CPU profile comparison (v2 → v3)

Component v2 samples v2 % v3 samples v3 % Change
DependencyMgmtImporter 442 43.0% 143 20.4% -68%
updateWithImportedFrom 354 34.4% 44 6.3% -88%
CopyOnWriteArraySet 68 6.6% 0 0.0% -100%
ModelValidator 71 6.9% 66 9.4% same
I/O (XML, Zip, Jar) 78 7.6% 86 12.3% same
Other 369 35.9% 406 57.9% same
Total CPU samples 1028 701 -32%

What was fixed

1. updateWithImportedFrom (was 35% of CPU)

Every BOM-imported dependency was rebuilt through Dependency.newBuilder(dep, true).importedFrom(loc).build() — triggering forceCopy (wraps all sub-lists into builders), build (unwraps back to immutable), and object pool interning — just to set one metadata field.

Added withImportedFrom(InputLocation) + private copy constructor to model.vm. Shares all model fields by reference, bypasses Builder and object pool entirely.

2. CopyOnWriteArraySet (was 6.6% of CPU)

ConsumerPomArtifactTransformer.deferDeleteFile() added to a CopyOnWriteArraySet<Path> → O(n) indexOfRange scan + array copy per add. With 4383 modules: O(n²). Replaced with ConcurrentHashMap.newKeySet() → O(1).

Wall-clock benchmark (4383 modules, mvn validate)

Wall-clock difference between v2 and v3 is within noise (~0.3s on a 14s benchmark) because:

  • BOM import runs once on the root POM, not on the critical path for parallel child processing
  • The parallel 4383-module build is I/O and scheduling dominated

The cumulative improvement from rc-6 baseline remains ~27% faster.

@gnodet
gnodet requested a review from cstamas August 1, 2026 14:32
@gnodet

gnodet commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

New commit: Store single model-object fields as sub-builders

Added commit 4ade9fe that extends the deferred-build pattern from list-of-model-object fields to single model-object fields (multiplicity=1).

What changed

model.vm — single model-object fields now use sub-builders:

Builder classes now store fields like Build, Reporting, DependencyManagement, Parent, Scm, Organization, Prerequisites, IssueManagement, CiManagement, DistributionManagement, PluginManagement, Activation, ActivationOS, ActivationFile, ActivationProperty, BuildBase, Relocation, Site, DeploymentRepository, RepositoryPolicy as X.Builder instead of immutable X. This means:

Before After
Build build; in Builder Build.Builder build; in Builder
Setter stores immutable directly Setter wraps: X.newBuilder(val, false)
Getter returns field directly Getter calls .build() on sub-builder
No mutable access New getModifiable*() returns X.Builder
build() short-circuit: identity check build() short-circuit: null check
forceCopy copies immutable ref forceCopy wraps into sub-builder

Pipeline stages updated to use getModifiable*():

  • DefaultModelPathTranslator: mutates Build.Builder / Reporting.Builder sub-builders in place — eliminates Build.newBuilder(build).xyz().build() intermediate allocations (10+ field mutations per call)
  • DefaultModelNormalizer: sets deduplicated plugins directly on Build.Builder via getModifiableBuild().plugins(...)
  • DefaultPluginConfigurationExpander: mutates Build.Builder and nested PluginManagement.Builder sub-builders in place
  • DefaultPluginManagementInjector: extracts merged plugin list and sets it directly on Build.Builder, refactored to expose mergeManagedPluginList() returning just the list

Impact

Each pipeline stage that modifies Build, Reporting, or PluginManagement now avoids:

  1. Materializing the immutable sub-object (no Build.newBuilder(build))
  2. Building a new immutable (no .build())
  3. Wrapping it back in the Model.Builder setter (no builder.build(newBuild))

On a 4383-module reactor, each module passes through ~7 pipeline stages. Previously, every stage that touched Build would: get immutable → create Builder → set fields → build() → set back on Model.Builder. Now they: get sub-builder → set fields. One allocation instead of three per stage.

🤖 Generated with Claude Code

@gnodet
gnodet force-pushed the optimize-model-building-pipeline-defer-dependency branch from 625b0b0 to 20afa1a Compare August 2, 2026 01:33
Store list and single-object fields as sub-builders in generated
Builder classes so the pipeline can thread a Model.Builder end-to-end
without materialising intermediate immutable objects.

Key changes:
- Builder list fields store Collection<X.Builder> internally; getters
  lazily build immutable snapshots only when read
- Single-object fields (e.g. Build, DependencyManagement) stored as
  sub-builders with getModifiable*() accessors for in-place mutation
- Add visitBuilder() to MavenTransformer and builder-accepting default
  methods on SPI interfaces (ModelNormalizer, ProfileInjector, etc.)
- Thread Model.Builder through readEffectiveModel pipeline stages to
  avoid build()/newBuilder() round-trips between stages
- Eliminate unnecessary builder.build() calls in pipeline stages that
  only pass the model to the next stage

Benchmark on 4384-POM reactor (mvn validate, 10 runs):
  rc6:  avg 52.8s  median 52.4s
  this: avg 31.4s  median 32.3s  → 40% faster

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet
gnodet force-pushed the optimize-model-building-pipeline-defer-dependency branch from e7827b5 to 9949734 Compare August 2, 2026 12:50
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.

1 participant