Skip to content

Commit 9008818

Browse files
toddbaerttoddbaert
authored
perf: add heap benchmark and reduce allocations (#1156)
* chore: add heap benchmark and reduce allocations Signed-off-by: Todd Baert <[email protected]>
1 parent b144763 commit 9008818

14 files changed

+551
-75
lines changed

‎CONTRIBUTING.md

+10
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,16 @@ to run alone:
3030
mvn test -P e2e
3131
```
3232

33+
## Benchmarking
34+
35+
There is a small JMH benchmark suite for testing allocations that can be run with:
36+
37+
```sh
38+
mvn -P benchmark test-compile jmh:benchmark -Djmh.f=1 -Djmh.prof='dev.openfeature.sdk.benchmark.AllocationProfiler'
39+
```
40+
41+
If you are concerned about the repercussions of a change on memory usage, run this an compare the results to the committed. `benchmark.txt` file.
42+
3343
## Releasing
3444

3545
See [releasing](./docs/release.md).

‎benchmark.txt

+239
Large diffs are not rendered by default.

‎pom.xml

+24-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
2-
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
1+
<project xmlns="http://maven.apache.org/POM/4.0.0"
2+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
33
<modelVersion>4.0.0</modelVersion>
44

55
<groupId>dev.openfeature</groupId>
@@ -11,7 +11,7 @@
1111
<maven.compiler.source>1.8</maven.compiler.source>
1212
<maven.compiler.target>${maven.compiler.source}</maven.compiler.target>
1313
<junit.jupiter.version>5.11.2</junit.jupiter.version>
14-
<!-- exclusion expression for e2e tests -->
14+
<!-- exclusion expression for e2e tests -->
1515
<testExclusions>**/e2e/*.java</testExclusions>
1616
<module-name>${project.groupId}.${project.artifactId}</module-name>
1717
</properties>
@@ -146,6 +146,13 @@
146146
<scope>test</scope>
147147
</dependency>
148148

149+
<dependency>
150+
<groupId>org.openjdk.jmh</groupId>
151+
<artifactId>jmh-core</artifactId>
152+
<version>1.37</version>
153+
<scope>test</scope>
154+
</dependency>
155+
149156
</dependencies>
150157

151158
<dependencyManagement>
@@ -473,7 +480,7 @@
473480
<version>3.10.1</version>
474481
<configuration>
475482
<failOnWarnings>true</failOnWarnings>
476-
<doclint>all,-missing</doclint> <!-- ignore missing javadoc, these are enforced with more customizability in the checkstyle plugin -->
483+
<doclint>all,-missing</doclint> <!-- ignore missing javadoc, these are enforced with more customizability in the checkstyle plugin -->
477484
</configuration>
478485
<executions>
479486
<execution>
@@ -507,6 +514,19 @@
507514
</build>
508515
</profile>
509516

517+
<profile>
518+
<id>benchmark</id>
519+
<build>
520+
<plugins>
521+
<plugin>
522+
<groupId>pw.krejci</groupId>
523+
<artifactId>jmh-maven-plugin</artifactId>
524+
<version>0.2.2</version>
525+
</plugin>
526+
</plugins>
527+
</build>
528+
</profile>
529+
510530
<profile>
511531
<id>e2e</id>
512532
<properties>

‎src/main/java/dev/openfeature/sdk/AbstractStructure.java

+6
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ abstract class AbstractStructure implements Structure {
88

99
protected final Map<String, Value> attributes;
1010

11+
@Override
12+
public boolean isEmpty() {
13+
return attributes == null || attributes.size() == 0;
14+
}
15+
1116
AbstractStructure() {
1217
this.attributes = new HashMap<>();
1318
}
@@ -32,4 +37,5 @@ public Map<String, Object> asObjectMap() {
3237
(accumulated, entry) -> accumulated.put(entry.getKey(), convertValue(entry.getValue())),
3338
HashMap::putAll);
3439
}
40+
3541
}

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

+27-32
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
package dev.openfeature.sdk;
22

3+
import java.util.ArrayList;
4+
import java.util.Collections;
35
import java.util.List;
46
import java.util.Map;
5-
import java.util.Objects;
67
import java.util.Optional;
78
import java.util.function.Consumer;
8-
import java.util.stream.Collectors;
9-
import java.util.stream.IntStream;
10-
import java.util.stream.Stream;
119

1210
import lombok.RequiredArgsConstructor;
1311
import lombok.extern.slf4j.Slf4j;
@@ -19,11 +17,7 @@ class HookSupport {
1917

2018
public EvaluationContext beforeHooks(FlagValueType flagValueType, HookContext hookCtx, List<Hook> hooks,
2119
Map<String, Object> hints) {
22-
Stream<EvaluationContext> result = callBeforeHooks(flagValueType, hookCtx, hooks, hints);
23-
return hookCtx.getCtx().merge(
24-
result.reduce(hookCtx.getCtx(), (EvaluationContext accumulated, EvaluationContext current) -> {
25-
return accumulated.merge(current);
26-
}));
20+
return callBeforeHooks(flagValueType, hookCtx, hooks, hints);
2721
}
2822

2923
public void afterHooks(FlagValueType flagValueType, HookContext hookContext, FlagEvaluationDetails details,
@@ -46,10 +40,11 @@ private <T> void executeHooks(
4640
String hookMethod,
4741
Consumer<Hook<T>> hookCode) {
4842
if (hooks != null) {
49-
hooks
50-
.stream()
51-
.filter(hook -> hook.supportsFlagValueType(flagValueType))
52-
.forEach(hook -> executeChecked(hook, hookCode, hookMethod));
43+
for (Hook hook : hooks) {
44+
if (hook.supportsFlagValueType(flagValueType)) {
45+
executeChecked(hook, hookCode, hookMethod);
46+
}
47+
}
5348
}
5449
}
5550

@@ -68,29 +63,29 @@ private <T> void executeHooksUnchecked(
6863
FlagValueType flagValueType, List<Hook> hooks,
6964
Consumer<Hook<T>> hookCode) {
7065
if (hooks != null) {
71-
hooks
72-
.stream()
73-
.filter(hook -> hook.supportsFlagValueType(flagValueType))
74-
.forEach(hookCode::accept);
66+
for (Hook hook : hooks) {
67+
if (hook.supportsFlagValueType(flagValueType)) {
68+
hookCode.accept(hook);
69+
}
70+
}
7571
}
7672
}
7773

78-
private Stream<EvaluationContext> callBeforeHooks(FlagValueType flagValueType, HookContext hookCtx,
74+
private EvaluationContext callBeforeHooks(FlagValueType flagValueType, HookContext hookCtx,
7975
List<Hook> hooks, Map<String, Object> hints) {
8076
// These traverse backwards from normal.
81-
List<Hook> reversedHooks = IntStream
82-
.range(0, hooks.size())
83-
.map(i -> hooks.size() - 1 - i)
84-
.mapToObj(hooks::get)
85-
.collect(Collectors.toList());
86-
87-
return reversedHooks
88-
.stream()
89-
.filter(hook -> hook.supportsFlagValueType(flagValueType))
90-
.map(hook -> hook.before(hookCtx, hints))
91-
.filter(Objects::nonNull)
92-
.filter(Optional::isPresent)
93-
.map(Optional::get)
94-
.map(EvaluationContext.class::cast);
77+
List<Hook> reversedHooks = new ArrayList<>(hooks);
78+
Collections.reverse(reversedHooks);
79+
EvaluationContext context = hookCtx.getCtx();
80+
for (Hook hook : reversedHooks) {
81+
if (hook.supportsFlagValueType(flagValueType)) {
82+
Optional<EvaluationContext> optional = Optional.ofNullable(hook.before(hookCtx, hints))
83+
.orElse(Optional.empty());
84+
if (optional.isPresent()) {
85+
context = context.merge(optional.get());
86+
}
87+
}
88+
}
89+
return context;
9590
}
9691
}

‎src/main/java/dev/openfeature/sdk/ImmutableContext.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,12 @@ public String getTargetingKey() {
7878
*/
7979
@Override
8080
public EvaluationContext merge(EvaluationContext overridingContext) {
81-
if (overridingContext == null) {
81+
if (overridingContext == null || overridingContext.isEmpty()) {
8282
return new ImmutableContext(this.asMap());
8383
}
84+
if (this.isEmpty()) {
85+
return new ImmutableContext(overridingContext.asMap());
86+
}
8487

8588
return new ImmutableContext(
8689
this.merge(ImmutableStructure::new, this.asMap(), overridingContext.asMap()));

‎src/main/java/dev/openfeature/sdk/ImmutableStructure.java

+14-18
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.util.HashMap;
44
import java.util.HashSet;
55
import java.util.Map;
6+
import java.util.Map.Entry;
67
import java.util.Optional;
78
import java.util.Set;
89

@@ -35,14 +36,7 @@ public ImmutableStructure() {
3536
* @param attributes attributes.
3637
*/
3738
public ImmutableStructure(Map<String, Value> attributes) {
38-
super(new HashMap<>(attributes.entrySet()
39-
.stream()
40-
.collect(HashMap::new,
41-
(accumulated, entry) -> accumulated.put(entry.getKey(),
42-
Optional.ofNullable(entry.getValue())
43-
.map(Value::clone)
44-
.orElse(null)),
45-
HashMap::putAll)));
39+
super(copyAttributes(attributes));
4640
}
4741

4842
@Override
@@ -53,7 +47,7 @@ public Set<String> keySet() {
5347
// getters
5448
@Override
5549
public Value getValue(String key) {
56-
Value value = this.attributes.get(key);
50+
Value value = attributes.get(key);
5751
return value != null ? value.clone() : null;
5852
}
5953

@@ -64,14 +58,16 @@ public Value getValue(String key) {
6458
*/
6559
@Override
6660
public Map<String, Value> asMap() {
67-
return attributes
68-
.entrySet()
69-
.stream()
70-
.collect(HashMap::new,
71-
(accumulated, entry) -> accumulated.put(entry.getKey(),
72-
Optional.ofNullable(entry.getValue())
73-
.map(Value::clone)
74-
.orElse(null)),
75-
HashMap::putAll);
61+
return copyAttributes(attributes);
7662
}
63+
64+
private static Map<String, Value> copyAttributes(Map<String, Value> in) {
65+
Map<String, Value> copy = new HashMap<>();
66+
for (Entry<String, Value> entry : in.entrySet()) {
67+
copy.put(entry.getKey(),
68+
Optional.ofNullable(entry.getValue()).map((Value val) -> val.clone()).orElse(null));
69+
}
70+
return copy;
71+
}
72+
7773
}

‎src/main/java/dev/openfeature/sdk/MutableContext.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,11 @@ public String getTargetingKey() {
114114
*/
115115
@Override
116116
public EvaluationContext merge(EvaluationContext overridingContext) {
117-
if (overridingContext == null) {
118-
return new MutableContext(this.asMap());
117+
if (overridingContext == null || overridingContext.isEmpty()) {
118+
return this;
119+
}
120+
if (this.isEmpty()) {
121+
return overridingContext;
119122
}
120123

121124
Map<String, Value> merged = this.merge(

‎src/main/java/dev/openfeature/sdk/MutableStructure.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@ public MutableStructure(Map<String, Value> attributes) {
3030

3131
@Override
3232
public Set<String> keySet() {
33-
return this.attributes.keySet();
33+
return attributes.keySet();
3434
}
3535

3636
// getters
3737
@Override
3838
public Value getValue(String key) {
39-
return this.attributes.get(key);
39+
return attributes.get(key);
4040
}
4141

4242
// adders
@@ -87,6 +87,6 @@ public MutableStructure add(String key, List<Value> value) {
8787
*/
8888
@Override
8989
public Map<String, Value> asMap() {
90-
return new HashMap<>(this.attributes);
90+
return new HashMap<>(attributes);
9191
}
9292
}

‎src/main/java/dev/openfeature/sdk/Structure.java

+14-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@
1818
@SuppressWarnings("PMD.BeanMembersShouldSerialize")
1919
public interface Structure {
2020

21+
/**
22+
* Boolean indicating if this structure is empty.
23+
* @return boolean for emptiness
24+
*/
25+
boolean isEmpty();
26+
2127
/**
2228
* Get all keys.
2329
*
@@ -113,7 +119,14 @@ default Object convertValue(Value value) {
113119
default <T extends Structure> Map<String, Value> merge(Function<Map<String, Value>, Structure> newStructure,
114120
Map<String, Value> base,
115121
Map<String, Value> overriding) {
116-
122+
123+
if (base.isEmpty()) {
124+
return overriding;
125+
}
126+
if (overriding.isEmpty()) {
127+
return base;
128+
}
129+
117130
final Map<String, Value> merged = new HashMap<>(base);
118131
for (Entry<String, Value> overridingEntry : overriding.entrySet()) {
119132
String key = overridingEntry.getKey();

‎src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java

+6-7
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
package dev.openfeature.sdk.internal;
22

3-
import java.util.Arrays;
4-
import java.util.Collection;
3+
import java.util.ArrayList;
54
import java.util.List;
65
import java.util.Map;
76
import java.util.function.Supplier;
8-
import java.util.stream.Collectors;
97

108
import lombok.experimental.UtilityClass;
119

@@ -64,9 +62,10 @@ public static <T> T defaultIfNull(T source, Supplier<T> defaultValue) {
6462
*/
6563
@SafeVarargs
6664
public static <T> List<T> merge(List<T>... sources) {
67-
return Arrays
68-
.stream(sources)
69-
.flatMap(Collection::stream)
70-
.collect(Collectors.toList());
65+
List<T> merged = new ArrayList<>();
66+
for (List<T> source : sources) {
67+
merged.addAll(source);
68+
}
69+
return merged;
7170
}
7271
}

0 commit comments

Comments
 (0)