feat: Layered context to reduce memory churn #1717

chrfwow · 2025-11-12T16:04:24Z

Introduces layered context to reduce memory churn

Part of #1718

Signed-off-by: christian.lutnik <[email protected]>

# Conflicts: # src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java

Signed-off-by: christian.lutnik <[email protected]>

codecov · 2025-11-12T16:11:49Z

Codecov Report

❌ Patch coverage is 94.93671% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.72%. Comparing base (060dd41) to head (499436b).
⚠️ Report is 68 commits behind head on main.

Files with missing lines	Patch %	Lines
.../dev/openfeature/sdk/LayeredEvaluationContext.java	94.61%	1 Missing and 6 partials ⚠️
src/main/java/dev/openfeature/sdk/HookSupport.java	75.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1717      +/-   ##
============================================
+ Coverage     91.69%   93.72%   +2.03%     
- Complexity      517      597      +80     
============================================
  Files            51       53       +2     
  Lines          1264     1403     +139     
  Branches        112      150      +38     
============================================
+ Hits           1159     1315     +156     
+ Misses           68       50      -18     
- Partials         37       38       +1

Flag	Coverage Δ
unittests	`93.72% <94.93%> (+2.03%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

Signed-off-by: christian.lutnik <[email protected]>

chrfwow · 2025-11-18T16:02:11Z

src/main/java/dev/openfeature/sdk/LayeredEvaluationContext.java

+ * API-level, Transaction-level, Client-level, Invocation-level, and Hook-level.
+ * The contexts are checked in that order for values, with Hook-level having the highest precedence.
+ */
+public class LayeredEvaluationContext implements EvaluationContext {


Alternatively, I could also implement this as a list of EvaluationContexts, with each list entry representing a context level (index 0 = api, index 1 = transaction, ...) This could maybe make the code cleaner (especially the merging, as I would just need to add another list entry; I don't think merge will/should be called often on this context though), but also less performant. Wdyt?

so theoretically before hooks can also add information for the evaluation context - and this should also be somehow merged into this. would it make sense to simple add all the evaluationContext returned to the layer, and iterate over them as a list?

Question if it is worth it - at least in theory it would be one less object

Signed-off-by: christian.lutnik <[email protected]>

chrfwow · 2025-11-19T08:46:19Z

Benchmark results:
(I ran AllocationBenchmark#main with the IntelliJ Profiler. This is not a proper JMH benchmark, so the values are not representative per evaluation, but should be helpful to compare both versions)

Baseline:
Duration: 5267 ms
Allocations: 16.27 GB
Flamegraph of allocations:

This PR:
Duration: 1649 ms
Allocations: 7.75 GB
Flamegraph of allocations:

aepfli

looks good to me, i only have a nit (regarding optionals) and a question for beforehooks :)

aepfli · 2025-11-19T09:42:41Z

src/main/java/dev/openfeature/sdk/HookSupport.java

            Optional<EvaluationContext> returnedEvalContext = Optional.ofNullable(
                            hook.before(hookContext, data.getHints()))
                    .orElse(Optional.empty());
            if (returnedEvalContext.isPresent()) {


As we are using Optionals already, we should use all the available syntax sugar, and switch to .ifPresent and chain as much as possible

That would be nice, but Optionals in Java don't work well enough to get this with type safety. I would need some castings and redundant isPresent checks to get this to work

but do we need the Optional wrapping on line 61? we want to reduce object churn etc. but we are generating a new Optional all the time - so i am wondering if we really need the Optional or if a null check would be sufficient here

I tried to change as little as possible of unrelated code so that we can scientifically see the benefit of the new context. Other optimizations will come in a later PR

aepfli · 2025-11-19T09:43:58Z

src/main/java/dev/openfeature/sdk/LayeredEvaluationContext.java

+ * API-level, Transaction-level, Client-level, Invocation-level, and Hook-level.
+ * The contexts are checked in that order for values, with Hook-level having the highest precedence.
+ */
+public class LayeredEvaluationContext implements EvaluationContext {


so theoretically before hooks can also add information for the evaluation context - and this should also be somehow merged into this. would it make sense to simple add all the evaluationContext returned to the layer, and iterate over them as a list?

Question if it is worth it - at least in theory it would be one less object

Signed-off-by: christian.lutnik <[email protected]>

# Conflicts: # src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmarkState.java

Signed-off-by: christian.lutnik <[email protected]>

src/main/java/dev/openfeature/sdk/LayeredEvaluationContext.java

toddbaert · 2025-11-20T00:58:51Z

src/main/java/dev/openfeature/sdk/LayeredEvaluationContext.java

+        if (apiContext != null) {
+            keys.addAll(apiContext.keySet());
+        }
+        this.keySet = keys;


Should we make use of the Collections.emptySet() singleton while we have no keys?

I don't think I understand your comment, we only know that we have no keys at the end of this method

I was saying perhaps we could more lazily create a key set if there actually is any keys, and otherwise just return the empty set and avoid allocating something, but it's a micro-optimization.

But how do we know if we have any keys without generating the keyset? And after we generated it, we might as well return it, as we have allocated it anyway

It's the allocation of the HashSet on 107 that I think we can save, possibly, which is a heap object... I think we can do all the null checks and then only create this object if any of them are non null, otherwise just return the singleton, right? Maybe I'm missing something.

could the isEmpty() method be used to determine whether to create the HashSet, or if true, return the singleton?

toddbaert · 2025-11-20T00:59:11Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a LayeredEvaluationContext to reduce memory churn during flag evaluations by avoiding the merging of evaluation contexts into a single map. The implementation looks solid and achieves its goal. I've added one suggestion to improve the performance of asMap(), asUnmodifiableMap(), and asObjectMap() methods in the new LayeredEvaluationContext class by optimizing how the final map is constructed. Overall, this is a great improvement for performance.

gemini-code-assist · 2025-11-20T01:01:51Z

src/main/java/dev/openfeature/sdk/LayeredEvaluationContext.java

+    public Map<String, Value> asMap() {
+        var keySet = ensureKeySet();
+        var keys = keySet.size();
+        if (keys == 0) {
+            return new HashMap<>(0);
+        }
+        var map = new HashMap<String, Value>(keys);
+
+        for (String key : keySet) {
+            map.put(key, getValue(key));
+        }
+        return map;
+    }
+
+    @Override
+    public Map<String, Value> asUnmodifiableMap() {
+        var keySet = ensureKeySet();
+        var keys = keySet.size();
+        if (keys == 0) {
+            return Collections.emptyMap();
+        }
+        var map = new HashMap<String, Value>(keys);
+
+        for (String key : keySet) {
+            map.put(key, getValue(key));
+        }
+        return Collections.unmodifiableMap(map);
+    }
+
+    @Override
+    public Map<String, Object> asObjectMap() {
+        var keySet = ensureKeySet();
+        var keys = keySet.size();
+        if (keys == 0) {
+            return new HashMap<>(0);
+        }
+        var map = new HashMap<String, Object>(keys);
+
+        for (String key : keySet) {
+            map.put(key, convertValue(getValue(key)));
+        }
+        return map;
+    }


The implementations of asMap(), asUnmodifiableMap(), and asObjectMap() can be made more efficient. The current approach first builds a complete keySet by iterating through all context layers, and then for each key, it calls getValue(), which again iterates through the layers to find the value. This results in multiple traversals of the context layers for each call to these methods.

A more performant approach would be to iterate through the context layers from lowest to highest precedence, adding all entries from each context to a result map. This way, values from higher-precedence contexts will naturally overwrite those from lower-precedence ones, and each context is traversed only once. This also aligns better with the goal of reducing memory churn by avoiding the creation of an intermediate keySet.

@Override public Map<String, Value> asMap() { var map = new HashMap<String, Value>(); // Apply from lowest to highest precedence, so that higher precedence contexts overwrite lower ones. if (apiContext != null) { map.putAll(apiContext.asMap()); } if (transactionContext != null) { map.putAll(transactionContext.asMap()); } if (clientContext != null) { map.putAll(clientContext.asMap()); } if (invocationContext != null) { map.putAll(invocationContext.asMap()); } if (hookContexts != null) { for (EvaluationContext hookContext : hookContexts) { if (hookContext != null) { map.putAll(hookContext.asMap()); } } } return map; } @Override public Map<String, Value> asUnmodifiableMap() { return Collections.unmodifiableMap(asMap()); } @Override public Map<String, Object> asObjectMap() { var map = new HashMap<String, Object>(); // Apply from lowest to highest precedence, so that higher precedence contexts overwrite lower ones. if (apiContext != null) { map.putAll(apiContext.asObjectMap()); } if (transactionContext != null) { map.putAll(transactionContext.asObjectMap()); } if (clientContext != null) { map.putAll(clientContext.asObjectMap()); } if (invocationContext != null) { map.putAll(invocationContext.asObjectMap()); } if (hookContexts != null) { for (EvaluationContext hookContext : hookContexts) { if (hookContext != null) { map.putAll(hookContext.asObjectMap()); } } } return map; }

That really helped a lot

toddbaert

@chrfwow looks excellent.

I've added some possible small optimizations, but no blockers.

Signed-off-by: christian.lutnik <[email protected]>

jarebudev · 2025-11-20T23:00:21Z

src/main/java/dev/openfeature/sdk/LayeredEvaluationContext.java

+
+    @Override
+    public boolean isEmpty() {
+        return (hookContexts == null || hookContexts.isEmpty())


If the hookContexts collection contains at least one EvaluationContext, then a call to isEmpty will return false.

But we do not check inside the hookContexts collection that all the EvaluationContexts that it contains are all empty.

should these also be checked for emptiness?

e.g. (hookContexts == null || hookContexts.isEmpty() || hookContexts.stream().allMatch(ec -> ec.isEmpty()))

Nice catch!

Signed-off-by: christian.lutnik <[email protected]>

toddbaert · 2025-11-25T15:50:25Z

I'll merge this tomorrow unless I hear objections.

Signed-off-by: christian.lutnik <[email protected]>

sonarqubecloud · 2025-11-25T16:13:31Z

Quality Gate passed

Issues
3 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

chrfwow added 7 commits September 15, 2025 10:58

add bench

df49024

Signed-off-by: christian.lutnik <[email protected]>

fix bench

ea3990d

Signed-off-by: christian.lutnik <[email protected]>

Merge branch 'refs/heads/main' into cache-baseline

1e7ee96

# Conflicts: # src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java

merge master

9d49b91

Signed-off-by: christian.lutnik <[email protected]>

improve benchmark

628eda6

Signed-off-by: christian.lutnik <[email protected]>

improve benchmark

2e9af46

Signed-off-by: christian.lutnik <[email protected]>

layered context

854b29f

Signed-off-by: christian.lutnik <[email protected]>

chrfwow mentioned this pull request Nov 12, 2025

Layered Evaluation Context #1718

Open

chrfwow changed the title ~~Layered context~~ feat: Layered context Nov 12, 2025

chrfwow added 4 commits November 18, 2025 11:10

Merge branch 'main' into layered-context

2d82083

Merge branch 'main' into layered-context

6a3dd25

add tests

a4f5dfe

Signed-off-by: christian.lutnik <[email protected]>

add merge

d930855

Signed-off-by: christian.lutnik <[email protected]>

chrfwow commented Nov 18, 2025

View reviewed changes

add tests

ff150e7

Signed-off-by: christian.lutnik <[email protected]>

chrfwow marked this pull request as ready for review November 19, 2025 08:34

chrfwow requested review from a team as code owners November 19, 2025 08:34

chrfwow requested review from alexandraoberaigner and guidobrei November 19, 2025 08:38

aepfli approved these changes Nov 19, 2025

View reviewed changes

chrfwow added 4 commits November 19, 2025 11:04

lazy init hook ctx

a650a20

Signed-off-by: christian.lutnik <[email protected]>

use list for hook ctx

2a923ba

Signed-off-by: christian.lutnik <[email protected]>

add str hook for ctx

b9fc776

Signed-off-by: christian.lutnik <[email protected]>

Merge branch 'cache-baseline' into layered-context

42e5e79

# Conflicts: # src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmarkState.java

chrfwow requested a review from aepfli November 19, 2025 10:47

spotless

58985e0

Signed-off-by: christian.lutnik <[email protected]>

chrfwow requested a review from a team November 19, 2025 10:48

spotless

1cf1eac

Signed-off-by: christian.lutnik <[email protected]>

toddbaert requested review from jarebudev and justinabrahms November 20, 2025 00:32

toddbaert reviewed Nov 20, 2025

View reviewed changes

src/main/java/dev/openfeature/sdk/LayeredEvaluationContext.java Show resolved Hide resolved

toddbaert reviewed Nov 20, 2025

View reviewed changes

src/main/java/dev/openfeature/sdk/LayeredEvaluationContext.java Show resolved Hide resolved

toddbaert reviewed Nov 20, 2025

View reviewed changes

toddbaert self-requested a review November 20, 2025 00:59

gemini-code-assist bot reviewed Nov 20, 2025

View reviewed changes

toddbaert approved these changes Nov 20, 2025

View reviewed changes

improve as map

9a134ca

Signed-off-by: christian.lutnik <[email protected]>

chrfwow requested a review from leakonvalinka November 20, 2025 07:55

chrfwow added 3 commits November 20, 2025 10:59

improve tests

92ce275

Signed-off-by: christian.lutnik <[email protected]>

spotless

a73b3f2

Signed-off-by: christian.lutnik <[email protected]>

remove deprectaed notice

4ea4c29

Signed-off-by: christian.lutnik <[email protected]>

jarebudev reviewed Nov 20, 2025

View reviewed changes

aepfli approved these changes Nov 21, 2025

View reviewed changes

chrfwow requested a review from jarebudev November 25, 2025 15:03

check if hook context is empty

dcd9420

Signed-off-by: christian.lutnik <[email protected]>

toddbaert changed the title ~~feat: Layered context~~ feat: Layered context to reduce memory churn Nov 25, 2025

revert comments

499436b

Signed-off-by: christian.lutnik <[email protected]>

feat: Layered context to reduce memory churn #1717

Are you sure you want to change the base?

feat: Layered context to reduce memory churn #1717

Conversation

chrfwow commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

chrfwow Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aepfli Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrfwow commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aepfli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aepfli Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jarebudev Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toddbaert commented Nov 20, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toddbaert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toddbaert commented Nov 25, 2025

Uh oh!

sonarqubecloud bot commented Nov 25, 2025

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

chrfwow commented Nov 12, 2025 •

edited

Loading

codecov bot commented Nov 12, 2025 •

edited

Loading

chrfwow Nov 18, 2025 •

edited

Loading

aepfli Nov 19, 2025 •

edited

Loading

chrfwow commented Nov 19, 2025 •

edited

Loading

aepfli Nov 19, 2025 •

edited

Loading

jarebudev Nov 20, 2025 •

edited

Loading