perf: avoid ImmutableStructure allocation for empty ImmutableContext#1972
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthrough
ChangesImmutableContext/ImmutableStructure allocation optimization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1972 +/- ##
============================================
+ Coverage 92.13% 93.19% +1.05%
- Complexity 662 669 +7
============================================
Files 59 59
Lines 1628 1631 +3
Branches 184 185 +1
============================================
+ Hits 1500 1520 +20
+ Misses 80 66 -14
+ Partials 48 45 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Tobias Ibounig <tobias.ibounig@dynatrace.com>
db2a77a to
af146d6
Compare
haha ya, I first looked at the raw numbers and I was shocked - but still, these improvements are valid! Thanks as always. |
|



This PR
ImmutableStructure.EMPTYsingleton (package-private)ImmutableContext()andImmutableContext(String)passCollections.emptyMap()instead ofnew HashMap<>()ImmutableContext(String targetingKey, Map attributes)reusesImmutableStructure.EMPTYwhentargetingKeyis null and attributes are emptyRelated Issues
None
Notes
Before-hook implementations commonly return
new ImmutableContext()to signal no context change. Each call previously allocated a throwawayHashMapas the constructor argument, then a secondHashMapinsideImmutableStructure. PassingCollections.emptyMap()eliminates the first allocation; reusingImmutableStructure.EMPTYeliminates both theImmutableStructureand its backing map entirely.The sharing should be fine:
ImmutableStructurenever mutatesattributes—getValueclones values,asMap/keySetreturn copies.The
ConstructorBranchestests guard thetargetingKey != nullbranch so that a future refactor cannot accidentally returnEMPTYfor a context that carries a targeting key.mainrun:+totalAllocatedInstancesrun:+totalAllocatedBytesFollow-up Tasks
benchmark.txtafter all are applied