gnodet commented on PR #12653:
URL: https://github.com/apache/maven/pull/12653#issuecomment-5150608706
## 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 443dcc5b51)
**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`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
