Skip to content

Optimize reactor sort, model pool, and phase comparator performance - #12652

Open
gnodet wants to merge 1 commit into
masterfrom
quick-fix/reactor-sort-and-model-pool-perf
Open

Optimize reactor sort, model pool, and phase comparator performance#12652
gnodet wants to merge 1 commit into
masterfrom
quick-fix/reactor-sort-and-model-pool-perf

Conversation

@gnodet

@gnodet gnodet commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

JFR profiling of a 4,383-module reactor build revealed three hotspots that together consume ~35% of CPU time. All three share the same root cause: O(n) linear scans used inside tight loops or comparators.

Fixes

  • DefaultGraphBuilder (23.5% CPU): Three call sites use result.sort(comparing(sortedProjects::indexOf)) where ArrayList.indexOf() is O(n), causing O(n² log n) total MavenProject.equals() calls (~230M for 4,383 modules). Replaced with a HashMap<MavenProject, Integer> index built in O(n), reducing each sort comparison to O(1).

  • DefaultModelObjectPool (~8% CPU, ~2.4 GB allocation pressure):

    • getPooledTypes() re-parses a comma-separated property string into a new StreamSet on every process() call (hundreds of thousands of times). Cached at construction time.
    • PoolKey.dependencyHashCode() uses Objects.hash() with 12 arguments, allocating a new Object[12] on every call. Inlined the hash computation.
    • Added hashCode fast-rejection to PoolKey.equals() to skip expensive deep equality when hashes differ.
  • PhaseComparator (~2% CPU): List.indexOf() in compare() is O(n) per call. Pre-built a HashMap<String, Integer> in the constructor for O(1) phase index lookups.

JFR Profile (before these fixes)

Rank Method CPU % Root Cause
1 MavenProject.equals 23.5% Called from ArrayList.indexOf in DefaultGraphBuilder sort
2 DefaultModelObjectPool$PoolKey.* ~8% Stream re-parse + Objects.hash varargs
3 DefaultDependencyManagementImporter.importManagement 5.8% (not addressed — model-level, not a data-structure issue)
4 PhaseComparator.compare ~2% List.indexOf per comparison

Complexity reduction

Component Before After
DefaultGraphBuilder sort O(n² log n) O(n log n)
DefaultModelObjectPool.process() O(k) stream + alloc per call O(1) Set.contains
PoolKey.dependencyHashCode 1 Object[12] alloc per call 0 allocations
PhaseComparator.compare O(n) per comparison O(1) per comparison

Test plan

  • DefaultModelObjectPoolTest passes
  • Full maven-impl test suite passes
  • DefaultGraphBuilderTest (blocked by pre-existing ProjectBuildLogAppender compile error on master — unrelated to this PR)
  • CI validation

🤖 Generated with Claude Code

…mance

JFR profiling of a 4,383-module reactor revealed three hotspots that
together consume ~35% of CPU time during dependency resolution:

1. DefaultGraphBuilder: result.sort(comparing(sortedProjects::indexOf))
   uses O(n) ArrayList.indexOf per comparison, causing O(n² log n) total
   MavenProject.equals calls (~230M for 4,383 modules). Replace with a
   HashMap<MavenProject, Integer> index built in O(n), reducing sort
   comparisons to O(1) each. Affects 3 call sites.

2. DefaultModelObjectPool.getPooledTypes(): re-parses a comma-separated
   property string into a new Stream → Set on every process() call.
   Cache the parsed Set at construction time. Also add hashCode
   fast-rejection to PoolKey.equals() to skip expensive deep equality
   when hashes differ.

3. DefaultModelObjectPool.PoolKey.dependencyHashCode(): Objects.hash()
   with 12 arguments allocates a new Object[12] on every call. Inline
   the hash computation to eliminate the varargs allocation.

4. PhaseComparator: List.indexOf() in compare() is O(n) per call.
   Pre-build a HashMap<String, Integer> in the constructor for O(1)
   phase index lookups.

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

@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, well-motivated performance optimization replacing O(n) linear scans with O(1) HashMap lookups in three hot paths identified by JFR profiling. All changes preserve semantic equivalence and are low risk.

Notable details:

  • The inlined dependencyHashCode computation is mathematically identical to Objects.hash(...), avoiding the varargs allocation.
  • The hash-code fast-rejection in PoolKey.equals() is a textbook optimization.
  • MavenProject has proper equals()/hashCode() based on GAV, confirming it is safe as a HashMap key.

No new tests needed since the changes are purely algorithmic (same inputs, same outputs, different time complexity).

Note: Cannot submit as APPROVE because the PR author matches the review account.

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

Claude Code on behalf of gnodet

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