FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

perf: avoid ImmutableStructure allocation for empty ImmutableContext by tobias-ibounig-dt · Pull Request #1972 · open-feature/java-sdk · GitHub

/ java-sdk Public

perf: avoid ImmutableStructure allocation for empty ImmutableContext - #1972

Merged
toddbaert merged 1 commit into
open-feature:mainfrom
tobias-ibounig-dt:perf/pr-9-avoid-immutable-structure-alloc
Jun 19, 2026
ConversationCommits (1)ChecksFiles changed
Merged

perf: avoid ImmutableStructure allocation for empty ImmutableContext#1972
toddbaert merged 1 commit into
open-feature:mainfrom
tobias-ibounig-dt:perf/pr-9-avoid-immutable-structure-alloc

Conversation

tobias-ibounig-dt commented Jun 19, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

This PR

  • Add ImmutableStructure.EMPTY singleton (package-private)
  • ImmutableContext() and ImmutableContext(String) pass Collections.emptyMap() instead of new HashMap<>()
  • ImmutableContext(String targetingKey, Map attributes) reuses ImmutableStructure.EMPTY when targetingKey is null and attributes are empty
  • Tests to guard the constructor branch logic

Related Issues

None

Notes

Before-hook implementations commonly return new ImmutableContext() to signal no context change. Each call previously allocated a throwaway HashMap as the constructor argument, then a second HashMap inside ImmutableStructure. Passing Collections.emptyMap() eliminates the first allocation; reusing ImmutableStructure.EMPTY eliminates both the ImmutableStructure and its backing map entirely.

The sharing should be fine: ImmutableStructure never mutates attributes — getValue clones values, asMap/keySet return copies.

The ConstructorBranches tests guard the targetingKey != null branch so that a future refactor cannot accidentally return EMPTY for a context that carries a targeting key.

Metric main This PR Delta
run:+totalAllocatedInstances 2,145,282 1,825,005 −320,277 (−14.9%)
run:+totalAllocatedBytes 102,105,728 93,695,320 −8,410,408 (−8.2%)

⚠️ seems I might have used another branch as baseline instead of main. Result proportions should be right, but not the exact values.

Follow-up Tasks

  • More PRs with memory improvements
  • Update benchmark.txt after all are applied

tobias-ibounig-dt requested review from a team as code owners June 19, 2026 11:40

coderabbitai Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d9dbeb10-cc4f-4351-8e62-cdc5aa0e68ef

📥 Commits

Reviewing files that changed from the base of the PR and between db2a77a and af146d6.

📒 Files selected for processing (3)
  • src/main/java/dev/openfeature/sdk/ImmutableContext.java
  • src/main/java/dev/openfeature/sdk/ImmutableStructure.java
  • src/test/java/dev/openfeature/sdk/ImmutableContextTest.java
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/main/java/dev/openfeature/sdk/ImmutableStructure.java
  • src/test/java/dev/openfeature/sdk/ImmutableContextTest.java
  • src/main/java/dev/openfeature/sdk/ImmutableContext.java
📝 Walkthrough

Walkthrough

ImmutableStructure gains a package-visible EMPTY static constant backed by Collections.emptyMap(). ImmutableContext constructors are updated to pass Collections.emptyMap() instead of allocating new HashMap<>(), and the two-arg constructor assigns ImmutableStructure.EMPTY when the targeting key is null and attributes are null or empty. Tests covering all four targeting-key/attributes constructor branches are added.

Changes

ImmutableContext/ImmutableStructure allocation optimization

Layer / File(s) Summary
ImmutableStructure.EMPTY constant
src/main/java/dev/openfeature/sdk/ImmutableStructure.java		 Adds java.util.Collections import and a package-visible static final EMPTY field initialized via ImmutableStructure(Collections.emptyMap()).
ImmutableContext constructor optimization and tests
src/main/java/dev/openfeature/sdk/ImmutableContext.java, src/test/java/dev/openfeature/sdk/ImmutableContextTest.java		 Removes HashMap import; no-arg and single-arg constructors delegate with Collections.emptyMap(); the two-arg constructor assigns ImmutableStructure.EMPTY when targetingKey is null and attributes are null or empty. A new @Nested test class (ConstructorBranches) validates all four combinations of null/non-null targeting key and empty/null attributes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • open-feature/java-sdk#1975: Updates NoOpTransactionContextPropagator.getTransactionContext() to return ImmutableContext.EMPTY singleton, complementing this PR's introduction of the EMPTY constant.

Suggested reviewers

  • toddbaert
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main optimization: avoiding unnecessary ImmutableStructure allocation for empty ImmutableContext instances.
Description check ✅ Passed The description provides comprehensive context for the changes, including the rationale, implementation details, performance metrics, and test coverage strategy.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Comment @coderabbitai help to get the list of available commands and usage tips.

codecov Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.19%. Comparing base (e014572) to head (af146d6).

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
Flag Coverage Δ
unittests 93.19% <100.00%> (+1.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

perf: avoid ImmutableStructure allocation for empty ImmutableContext
af146d6 
Signed-off-by: Tobias Ibounig <tobias.ibounig@dynatrace.com>
toddbaert force-pushed the perf/pr-9-avoid-immutable-structure-alloc branch from db2a77a to af146d6 Compare June 19, 2026 18:41

toddbaert commented Jun 19, 2026

Copy link
Copy Markdown
Member

⚠️ seems I might have used another branch as baseline instead of main. Result proportions should be right, but not the exact values.

haha ya, I first looked at the raw numbers and I was shocked - but still, these improvements are valid! Thanks as always.

toddbaert merged commit b54f6dc into open-feature:main Jun 19, 2026
12 checks passed
openfeaturebot mentioned this pull request Jun 19, 2026
Merged

sonarqubecloud Bot commented Jun 19, 2026

Copy link
Copy Markdown

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

coderabbitai Bot mentioned this pull request Jun 22, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

toddbaert toddbaert approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL