Skip to content

[MNG-8507] Forward port: Reduce allocation pressure in model building pipeline - #12553

Merged
gnodet merged 2 commits into
masterfrom
forward-port/mng-8507-alloc-pressure
Jul 27, 2026
Merged

[MNG-8507] Forward port: Reduce allocation pressure in model building pipeline#12553
gnodet merged 2 commits into
masterfrom
forward-port/mng-8507-alloc-pressure

Conversation

@gnodet

@gnodet gnodet commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Forward port of #12540 (merged to maven-4.0.x) to master.

Cherry-picked from f5426bf with conflict resolution in:

  • DefaultModelValidator.java — adapted variable names (session vs s, dependency vs d) and FQN type references (org.eclipse.aether.scope.DependencyScope)
  • TypeRegistryAdapter.java — kept public visibility (class moved to a different package on master)

See #12540 for the full description and JFR profiling results.

Test plan

  • Compiles cleanly on master
  • CI: full-build + integration-tests matrix

🤖 Generated with Claude Code

Reduce allocation pressure in the Maven 4 immutable model building
pipeline, measured with JFR on Apache Camel (676 modules).
Overall allocation reduction: ~49% (from ~4,000 MB to ~2,054 MB).

Key optimizations:
- DefaultModelInterpolator: skip interpolation callback for strings
  without '$' via anonymous MavenTransformer subclass override
- DefaultModelInterpolator: share HashSet for cycle detection across
  all strings in a model, avoiding ~550 allocations per module
- DefaultInterpolator.unescape(): fast-path when no escape markers
- DefaultModelValidator: hoist scope computation before dep loop
- InternalSession.from(): instanceof fast-path before string concat
- MavenProject.hashCode(): inline hash to avoid varargs Object[]
- TypeRegistryAdapter: cache get() results in ConcurrentHashMap
- PropertiesAsMap: eliminate Entry wrapper allocation, replace
  stream().filter().count() with imperative loop
- ReflectionValueExtractor: make accessor prefix list a static
  constant instead of per-call Arrays.asList()
- transformer.vm: add null-check in transform(String)

Closes #12540

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Clean forward port of MNG-8507 allocation-pressure optimizations from maven-4.0.x to master. All ten files are correct; the conflict resolutions (variable names, FQN types, visibility) are appropriate. No functional regressions detected.

Key observations:

  • The optimizations are well-targeted at JFR-identified hotspots (cycleMap allocation, scope validation redundancy, wrapper entry objects, varargs arrays, stream pipelines). All changes preserve existing semantics.
  • The null-check addition in transformer.vm (transform(String)) is not just a performance optimization but a correctness fix — it prevents potential NPE when model string fields are null.
  • The HashSet reuse for cycle detection in DefaultModelInterpolator is thread-safe because MavenTransformer.visit() processes strings sequentially in a single thread.

Minor notes (non-blocking):

  • The cast (DefaultInterpolator) interpolator in DefaultModelInterpolator couples to a concrete implementation — consider adding a brief comment explaining why the downcast is safe and necessary.
  • The informational comment about Flex Mojos custom scopes in DefaultModelValidator was removed during refactoring — consider preserving it near the scope validation logic for historical context.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

gnodet added a commit to gnodet/maven that referenced this pull request Jul 27, 2026
…ment

- DefaultModelInterpolator: explain why the (DefaultInterpolator) cast
  is safe and necessary (access package-private overload for HashSet reuse)
- DefaultModelValidator: restore historical comment about Flex Mojos
  custom scopes near the scope validation logic

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet

gnodet commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both notes in 9eff25c:

  1. DefaultInterpolator downcast — added comment explaining why the (DefaultInterpolator) cast is safe (only implementation bound via DI) and necessary (access package-private overload accepting an external cycleMap).

  2. Flex Mojos comment — restored the historical comment about custom scopes near the scope validation call.

gnodet added a commit to gnodet/maven that referenced this pull request Jul 27, 2026
@gnodet
gnodet merged commit 2cbce40 into master Jul 27, 2026
23 checks passed
@gnodet
gnodet deleted the forward-port/mng-8507-alloc-pressure branch July 27, 2026 22:25
@github-actions

Copy link
Copy Markdown

@gnodet Please assign appropriate label to PR according to the type of change.

@github-actions github-actions Bot added this to the 4.1.0 milestone Jul 27, 2026
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