Skip to content

Optimize computeLocations() and simplify ModelObjectPool location handling - #13031

Merged
gnodet merged 1 commit into
masterfrom
optimize-locations-and-pool
Sep 4, 2026
Merged

Optimize computeLocations() and simplify ModelObjectPool location handling#13031
gnodet merged 1 commit into
masterfrom
optimize-locations-and-pool

Conversation

@gnodet

@gnodet gnodet commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Two targeted optimizations in the model building pipeline:

1. computeLocations() — replace Stream with HashMap (model.vm)

Replace Stream.concat().collect(Collectors.toUnmodifiableMap(...)) with HashMap.putAll() + Map.copyOf(), reducing intermediate KeyValueHolder allocations from stream-based map construction.

Also return oldlocs directly when newlocs is empty — the base locations map is already immutable, so the Map.copyOf() was redundant.

2. Add getLocations() accessor and simplify DefaultModelObjectPool (model.vm + DefaultModelObjectPool.java)

The original PoolKey code used getLocationKeys() + getLocation(key) to iterate locations entry by entry because the model had no direct map accessor. This required:

  • A separate locationsEqual() method iterating keys one by one
  • A separate locationsHashCode() method doing the same for hashing
  • A redundant Objects.equals(dep1.getLocationKeys(), dep2.getLocationKeys()) check

Fix: add getLocations() to the generated model, then replace all of the above with:

  • Objects.equals(dep1.getLocations(), dep2.getLocations()) for equality
  • dep.getLocations().hashCode() for hashing

Changes

File Insertions Deletions Net
src/mdo/model.vm +17 -4 +13
DefaultModelObjectPool.java +2 -38 -36
Total +19 -42 -23

@gnodet
gnodet force-pushed the optimize-locations-and-pool branch 2 times, most recently from 3557ede to 5439d15 Compare September 3, 2026 08:14
@gnodet gnodet changed the title Optimize computeLocations() and ModelObjectPool location comparison Optimize computeLocations() and simplify ModelObjectPool location handling Sep 3, 2026
@gnodet gnodet added this to the 4.1.0 milestone Sep 3, 2026

@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. Both changes — the computeLocations() stream removal and ModelObjectPool simplification via getLocations() — are semantically equivalent to the original behavior. The immutability invariant of the locations map is preserved across all code paths.

Minor: The java.util.stream.Collectors import in model.vm appears to be unused after this change — the only usage was in the computeLocations() stream pipeline (Collectors.toUnmodifiableMap) which is now replaced with HashMap.putAll() + Map.copyOf(). (java.util.stream.Stream is still needed for getLocationKeyStream().)

📋 PR Metadata

Aspect Current Suggested
Labels chore, performance Consider removing chore — this is a performance optimization, and the performance label is already the better fit

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

Claude Code on behalf of Guillaume Nodet

Two targeted optimizations in the model building pipeline:

1. computeLocations(): replace Stream.concat().collect(toUnmodifiableMap)
   with HashMap.putAll() + Map.copyOf(). Also return oldlocs directly
   when newlocs is empty (avoids unnecessary Map.copyOf since base
   locations are already immutable). Add getLocations() accessor for
   direct map access.

2. ModelObjectPool.PoolKey: the original code used getLocationKeys() +
   getLocation(key) to iterate locations because getLocations() did not
   exist. Add getLocations() to the model and use Objects.equals() on the
   map directly, replacing both the per-key iteration in locationsEqual()
   and the separate locationsHashCode() helper. Remove redundant
   getLocationKeys() equality check (subsumed by map equality).
@gnodet
gnodet force-pushed the optimize-locations-and-pool branch from 5439d15 to dc10131 Compare September 3, 2026 12:41
@gnodet
gnodet merged commit cfbbe07 into master Sep 4, 2026
23 checks passed
@gnodet
gnodet deleted the optimize-locations-and-pool branch September 4, 2026 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants