Skip to content

Commit 73aa53a

Browse files
perf: eliminate merge allocation in setHooks by accepting hook sources directly
Signed-off-by: Tobias Ibounig <tobias.ibounig@dynatrace.com>
1 parent 58091f3 commit 73aa53a

3 files changed

Lines changed: 67 additions & 14 deletions

File tree

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

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package dev.openfeature.sdk;
22

33
import java.util.ArrayList;
4+
import java.util.Collection;
45
import java.util.Collections;
56
import java.util.List;
67
import java.util.Optional;
@@ -16,19 +17,37 @@ class HookSupport {
1617
/**
1718
* Sets the {@link Hook}-{@link HookContext}-{@link Pair} list in the given data object with {@link HookContext}
1819
* set to null. Filters hooks by supported {@link FlagValueType}.
20+
* Sources are iterated in order: provider → options → client → API.
1921
*
2022
* @param hookSupportData the data object to modify
21-
* @param hooks the hooks to set
23+
* @param providerHooks provider-level hooks
24+
* @param optionHooks per-evaluation option hooks
25+
* @param clientHooks client-level hooks
26+
* @param apiHooks API-level hooks
2227
* @param type the flag value type to filter unsupported hooks
2328
*/
24-
public void setHooks(HookSupportData hookSupportData, List<Hook> hooks, FlagValueType type) {
29+
public void setHooks(
30+
HookSupportData hookSupportData,
31+
Collection<Hook> providerHooks,
32+
Collection<Hook> optionHooks,
33+
Collection<Hook> clientHooks,
34+
Collection<Hook> apiHooks,
35+
FlagValueType type) {
2536
List<Pair<Hook, HookContext>> hookContextPairs = new ArrayList<>();
26-
for (Hook hook : hooks) {
37+
addFilteredHooks(hookContextPairs, providerHooks, type);
38+
addFilteredHooks(hookContextPairs, optionHooks, type);
39+
addFilteredHooks(hookContextPairs, clientHooks, type);
40+
addFilteredHooks(hookContextPairs, apiHooks, type);
41+
hookSupportData.hooks = hookContextPairs;
42+
}
43+
44+
private static void addFilteredHooks(
45+
List<Pair<Hook, HookContext>> dest, Collection<Hook> source, FlagValueType type) {
46+
for (Hook hook : source) {
2747
if (hook.supportsFlagValueType(type)) {
28-
hookContextPairs.add(Pair.of(hook, null));
48+
dest.add(Pair.of(hook, null));
2949
}
3050
}
31-
hookSupportData.hooks = hookContextPairs;
3251
}
3352

3453
/**

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import dev.openfeature.sdk.exceptions.GeneralError;
66
import dev.openfeature.sdk.exceptions.OpenFeatureError;
77
import dev.openfeature.sdk.exceptions.ProviderNotReadyError;
8-
import dev.openfeature.sdk.internal.ObjectUtils;
98
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
109
import java.util.ArrayList;
1110
import java.util.Arrays;
@@ -185,9 +184,13 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
185184
final var state = stateManager.getState();
186185

187186
// Hooks are initialized as early as possible to enable the execution of error stages
188-
var mergedHooks = ObjectUtils.merge(
189-
provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, openfeatureApi.getMutableHooks());
190-
hookSupport.setHooks(hookSupportData, mergedHooks, type);
187+
hookSupport.setHooks(
188+
hookSupportData,
189+
provider.getProviderHooks(),
190+
flagOptions.getHooks(),
191+
clientHooks,
192+
openfeatureApi.getMutableHooks(),
193+
type);
191194

192195
var sharedHookContext =
193196
new SharedHookContext(key, type, this.getMetadata(), provider.getMetadata(), defaultValue);

src/test/java/dev/openfeature/sdk/HookSupportTest.java

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import dev.openfeature.sdk.fixtures.HookFixtures;
1111
import java.util.Arrays;
12+
import java.util.Collections;
1213
import java.util.HashMap;
1314
import java.util.List;
1415
import java.util.Map;
@@ -38,7 +39,13 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() {
3839
var sharedContext = getBaseHookContextForType(FlagValueType.STRING);
3940
var hookSupportData = new HookSupportData();
4041
hookSupportData.evaluationContext = layered;
41-
hookSupport.setHooks(hookSupportData, Arrays.asList(hook1, hook2), FlagValueType.STRING);
42+
hookSupport.setHooks(
43+
hookSupportData,
44+
Collections.emptyList(),
45+
Collections.emptyList(),
46+
Arrays.asList(hook1, hook2),
47+
Collections.emptyList(),
48+
FlagValueType.STRING);
4249
hookSupport.setHookContexts(hookSupportData, sharedContext, layered);
4350

4451
hookSupport.executeBeforeHooks(hookSupportData);
@@ -57,7 +64,13 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
5764
Hook<?> genericHook = mockGenericHook();
5865

5966
var hookSupportData = new HookSupportData();
60-
hookSupport.setHooks(hookSupportData, List.of(genericHook), flagValueType);
67+
hookSupport.setHooks(
68+
hookSupportData,
69+
Collections.emptyList(),
70+
Collections.emptyList(),
71+
List.of(genericHook),
72+
Collections.emptyList(),
73+
flagValueType);
6174

6275
callAllHooks(hookSupportData);
6376

@@ -73,7 +86,13 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
7386
void shouldPassDataAcrossStages(FlagValueType flagValueType) {
7487
var testHook = new TestHookWithData();
7588
var hookSupportData = new HookSupportData();
76-
hookSupport.setHooks(hookSupportData, List.of(testHook), flagValueType);
89+
hookSupport.setHooks(
90+
hookSupportData,
91+
Collections.emptyList(),
92+
Collections.emptyList(),
93+
List.of(testHook),
94+
Collections.emptyList(),
95+
flagValueType);
7796
hookSupport.setHookContexts(
7897
hookSupportData,
7998
getBaseHookContextForType(flagValueType),
@@ -102,7 +121,13 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) {
102121
var testHook2 = new TestHookWithData(2);
103122

104123
var hookSupportData = new HookSupportData();
105-
hookSupport.setHooks(hookSupportData, List.of(testHook1, testHook2), flagValueType);
124+
hookSupport.setHooks(
125+
hookSupportData,
126+
Collections.emptyList(),
127+
Collections.emptyList(),
128+
List.of(testHook1, testHook2),
129+
Collections.emptyList(),
130+
flagValueType);
106131
hookSupport.setHookContexts(
107132
hookSupportData,
108133
getBaseHookContextForType(flagValueType),
@@ -132,7 +157,13 @@ public Optional<EvaluationContext> before(HookContext ctx, Map hints) {
132157
var layeredEvaluationContext =
133158
new LayeredEvaluationContext(evaluationContextWithValue("key", "value"), null, null, null);
134159
hookSupportData.evaluationContext = layeredEvaluationContext;
135-
hookSupport.setHooks(hookSupportData, List.of(recursiveHook, emptyHook), FlagValueType.STRING);
160+
hookSupport.setHooks(
161+
hookSupportData,
162+
Collections.emptyList(),
163+
Collections.emptyList(),
164+
List.of(recursiveHook, emptyHook),
165+
Collections.emptyList(),
166+
FlagValueType.STRING);
136167
hookSupport.setHookContexts(
137168
hookSupportData, getBaseHookContextForType(FlagValueType.STRING), layeredEvaluationContext);
138169

0 commit comments

Comments
 (0)