perf: add heap benchmark and reduce allocations (#1156)

* chore: add heap benchmark and reduce allocations

Signed-off-by: Todd Baert <[email protected]>

CONTRIBUTING.md

@@ -30,6 +30,16 @@ to run alone:
 mvn test -P e2e
 ```
 
+## Benchmarking
+
+There is a small JMH benchmark suite for testing allocations that can be run with:
+
+```sh
+mvn -P benchmark test-compile jmh:benchmark -Djmh.f=1  -Djmh.prof='dev.openfeature.sdk.benchmark.AllocationProfiler'
+```
+
+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.
+
 ## Releasing
 
 See [releasing](./docs/release.md).

pom.xml

@@ -1,5 +1,5 @@
-<project xmlns="http://maven.apache.org/POM/4.0.0" 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">
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+  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">
   <modelVersion>4.0.0</modelVersion>
 
   <groupId>dev.openfeature</groupId>
@@ -11,7 +11,7 @@
     <maven.compiler.source>1.8</maven.compiler.source>
     <maven.compiler.target>${maven.compiler.source}</maven.compiler.target>
     <junit.jupiter.version>5.11.2</junit.jupiter.version>
-     <!-- exclusion expression for e2e tests -->
+    <!-- exclusion expression for e2e tests -->
     <testExclusions>**/e2e/*.java</testExclusions>
     <module-name>${project.groupId}.${project.artifactId}</module-name>
   </properties>
@@ -146,6 +146,13 @@
       <scope>test</scope>
     </dependency>
 
+    <dependency>
+      <groupId>org.openjdk.jmh</groupId>
+      <artifactId>jmh-core</artifactId>
+      <version>1.37</version>
+      <scope>test</scope>
+    </dependency>
+
   </dependencies>
 
   <dependencyManagement>
@@ -473,7 +480,7 @@
             <version>3.10.1</version>
             <configuration>
               <failOnWarnings>true</failOnWarnings>
-              <doclint>all,-missing</doclint>  <!-- ignore missing javadoc, these are enforced with more customizability in the checkstyle plugin -->
+              <doclint>all,-missing</doclint> <!-- ignore missing javadoc, these are enforced with more customizability in the checkstyle plugin -->
             </configuration>
             <executions>
               <execution>
@@ -507,6 +514,19 @@
       </build>
     </profile>
 
+    <profile>
+      <id>benchmark</id>
+      <build>
+        <plugins>
+          <plugin>
+            <groupId>pw.krejci</groupId>
+            <artifactId>jmh-maven-plugin</artifactId>
+            <version>0.2.2</version>
+          </plugin>
+        </plugins>
+      </build>
+    </profile>
+
     <profile>
       <id>e2e</id>
       <properties>

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

@@ -8,6 +8,11 @@ abstract class AbstractStructure implements Structure {
 
     protected final Map<String, Value> attributes;
 
+    @Override
+    public boolean isEmpty() {
+        return attributes == null || attributes.size() == 0;
+    }
+
     AbstractStructure() {
         this.attributes = new HashMap<>();
     }
@@ -32,4 +37,5 @@ public Map<String, Object> asObjectMap() {
                         (accumulated, entry) -> accumulated.put(entry.getKey(), convertValue(entry.getValue())),
                         HashMap::putAll);
     }
+
 }

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

@@ -1,13 +1,11 @@
 package dev.openfeature.sdk;
 
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Consumer;
-import java.util.stream.Collectors;
-import java.util.stream.IntStream;
-import java.util.stream.Stream;
 
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
@@ -19,11 +17,7 @@ class HookSupport {
 
     public EvaluationContext beforeHooks(FlagValueType flagValueType, HookContext hookCtx, List<Hook> hooks,
             Map<String, Object> hints) {
-        Stream<EvaluationContext> result = callBeforeHooks(flagValueType, hookCtx, hooks, hints);
-        return hookCtx.getCtx().merge(
-                result.reduce(hookCtx.getCtx(), (EvaluationContext accumulated, EvaluationContext current) -> {
-                    return accumulated.merge(current);
-                }));
+        return callBeforeHooks(flagValueType, hookCtx, hooks, hints);
     }
 
     public void afterHooks(FlagValueType flagValueType, HookContext hookContext, FlagEvaluationDetails details,
@@ -46,10 +40,11 @@ private <T> void executeHooks(
             String hookMethod,
             Consumer<Hook<T>> hookCode) {
         if (hooks != null) {
-            hooks
-                    .stream()
-                    .filter(hook -> hook.supportsFlagValueType(flagValueType))
-                    .forEach(hook -> executeChecked(hook, hookCode, hookMethod));
+            for (Hook hook : hooks) {
+                if (hook.supportsFlagValueType(flagValueType)) {
+                    executeChecked(hook, hookCode, hookMethod);
+                }
+            }
         }
     }
 
@@ -68,29 +63,29 @@ private <T> void executeHooksUnchecked(
             FlagValueType flagValueType, List<Hook> hooks,
             Consumer<Hook<T>> hookCode) {
         if (hooks != null) {
-            hooks
-                    .stream()
-                    .filter(hook -> hook.supportsFlagValueType(flagValueType))
-                    .forEach(hookCode::accept);
+            for (Hook hook : hooks) {
+                if (hook.supportsFlagValueType(flagValueType)) {
+                    hookCode.accept(hook);
+                }
+            }
         }
     }
 
-    private Stream<EvaluationContext> callBeforeHooks(FlagValueType flagValueType, HookContext hookCtx,
+    private EvaluationContext callBeforeHooks(FlagValueType flagValueType, HookContext hookCtx,
             List<Hook> hooks, Map<String, Object> hints) {
         // These traverse backwards from normal.
-        List<Hook> reversedHooks = IntStream
-                .range(0, hooks.size())
-                .map(i -> hooks.size() - 1 - i)
-                .mapToObj(hooks::get)
-                .collect(Collectors.toList());
-
-        return reversedHooks
-                .stream()
-                .filter(hook -> hook.supportsFlagValueType(flagValueType))
-                .map(hook -> hook.before(hookCtx, hints))
-                .filter(Objects::nonNull)
-                .filter(Optional::isPresent)
-                .map(Optional::get)
-                .map(EvaluationContext.class::cast);
+        List<Hook> reversedHooks = new ArrayList<>(hooks);
+        Collections.reverse(reversedHooks);
+        EvaluationContext context = hookCtx.getCtx();
+        for (Hook hook : reversedHooks) {
+            if (hook.supportsFlagValueType(flagValueType)) {
+                Optional<EvaluationContext> optional = Optional.ofNullable(hook.before(hookCtx, hints))
+                        .orElse(Optional.empty());
+                if (optional.isPresent()) {
+                    context = context.merge(optional.get());
+                }
+            }
+        }
+        return context;
     }
 }

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

@@ -78,9 +78,12 @@ public String getTargetingKey() {
      */
     @Override
     public EvaluationContext merge(EvaluationContext overridingContext) {
-        if (overridingContext == null) {
+        if (overridingContext == null || overridingContext.isEmpty()) {
             return new ImmutableContext(this.asMap());
         }
+        if (this.isEmpty()) {
+            return new ImmutableContext(overridingContext.asMap());
+        }
 
         return new ImmutableContext(
                 this.merge(ImmutableStructure::new, this.asMap(), overridingContext.asMap()));

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

@@ -3,6 +3,7 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.Set;
 
@@ -35,14 +36,7 @@ public ImmutableStructure() {
      * @param attributes attributes.
      */
     public ImmutableStructure(Map<String, Value> attributes) {
-        super(new HashMap<>(attributes.entrySet()
-                .stream()
-                .collect(HashMap::new,
-                        (accumulated, entry) -> accumulated.put(entry.getKey(),
-                                Optional.ofNullable(entry.getValue())
-                                    .map(Value::clone)
-                                    .orElse(null)),
-                        HashMap::putAll)));
+        super(copyAttributes(attributes));
     }
 
     @Override
@@ -53,7 +47,7 @@ public Set<String> keySet() {
     // getters
     @Override
     public Value getValue(String key) {
-        Value value = this.attributes.get(key);
+        Value value = attributes.get(key);
         return value != null ? value.clone() : null;
     }
 
@@ -64,14 +58,16 @@ public Value getValue(String key) {
      */
     @Override
     public Map<String, Value> asMap() {
-        return attributes
-                .entrySet()
-                .stream()
-                .collect(HashMap::new,
-                        (accumulated, entry) -> accumulated.put(entry.getKey(),
-                                Optional.ofNullable(entry.getValue())
-                                .map(Value::clone)
-                                .orElse(null)),
-                        HashMap::putAll);
+        return copyAttributes(attributes);
     }
+
+    private static Map<String, Value> copyAttributes(Map<String, Value> in) {
+        Map<String, Value> copy = new HashMap<>();
+        for (Entry<String, Value> entry : in.entrySet()) {
+            copy.put(entry.getKey(),
+                    Optional.ofNullable(entry.getValue()).map((Value val) -> val.clone()).orElse(null));
+        }
+        return copy;
+    }
+
 }

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

@@ -114,8 +114,11 @@ public String getTargetingKey() {
      */
     @Override
     public EvaluationContext merge(EvaluationContext overridingContext) {
-        if (overridingContext == null) {
-            return new MutableContext(this.asMap());
+        if (overridingContext == null || overridingContext.isEmpty()) {
+            return this;
+        }
+        if (this.isEmpty()) {
+            return overridingContext;
         }
 
         Map<String, Value> merged = this.merge(

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

@@ -30,13 +30,13 @@ public MutableStructure(Map<String, Value> attributes) {
 
     @Override
     public Set<String> keySet() {
-        return this.attributes.keySet();
+        return attributes.keySet();
     }
 
     // getters
     @Override
     public Value getValue(String key) {
-        return this.attributes.get(key);
+        return attributes.get(key);
     }
 
     // adders
@@ -87,6 +87,6 @@ public MutableStructure add(String key, List<Value> value) {
      */
     @Override
     public Map<String, Value> asMap() {
-        return new HashMap<>(this.attributes);
+        return new HashMap<>(attributes);
     }
 }

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

@@ -18,6 +18,12 @@
 @SuppressWarnings("PMD.BeanMembersShouldSerialize")
 public interface Structure {
 
+    /**
+     * Boolean indicating if this structure is empty.
+     * @return boolean for emptiness
+     */
+    boolean isEmpty();
+
     /**
      * Get all keys.
      *
@@ -113,7 +119,14 @@ default Object convertValue(Value value) {
     default <T extends Structure> Map<String, Value> merge(Function<Map<String, Value>, Structure> newStructure,
                                                            Map<String, Value> base,
                                                            Map<String, Value> overriding) {
-
+
+        if (base.isEmpty()) {
+            return overriding;
+        }
+        if (overriding.isEmpty()) {
+            return base;
+        }
+
         final Map<String, Value> merged = new HashMap<>(base);
         for (Entry<String, Value> overridingEntry : overriding.entrySet()) {
             String key = overridingEntry.getKey();

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

@@ -1,11 +1,9 @@
 package dev.openfeature.sdk.internal;
 
-import java.util.Arrays;
-import java.util.Collection;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Supplier;
-import java.util.stream.Collectors;
 
 import lombok.experimental.UtilityClass;
 
@@ -64,9 +62,10 @@ public static <T> T defaultIfNull(T source, Supplier<T> defaultValue) {
      */
     @SafeVarargs
     public static <T> List<T> merge(List<T>... sources) {
-        return Arrays
-            .stream(sources)
-            .flatMap(Collection::stream)
-            .collect(Collectors.toList());
+        List<T> merged = new ArrayList<>();
+        for (List<T> source : sources) {
+            merged.addAll(source);
+        }
+        return merged;
     }
 }