From 90088188c90da9fcca4e50328405e56f9ae17dde Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 15 Oct 2024 09:47:51 -0400 Subject: [PATCH] perf: add heap benchmark and reduce allocations (#1156) * chore: add heap benchmark and reduce allocations Signed-off-by: Todd Baert --- CONTRIBUTING.md | 10 + benchmark.txt | 239 ++++++++++++++++++ pom.xml | 28 +- .../openfeature/sdk/AbstractStructure.java | 6 + .../java/dev/openfeature/sdk/HookSupport.java | 59 ++--- .../dev/openfeature/sdk/ImmutableContext.java | 5 +- .../openfeature/sdk/ImmutableStructure.java | 32 +-- .../dev/openfeature/sdk/MutableContext.java | 7 +- .../dev/openfeature/sdk/MutableStructure.java | 6 +- .../java/dev/openfeature/sdk/Structure.java | 15 +- .../openfeature/sdk/internal/ObjectUtils.java | 13 +- .../sdk/benchmark/AllocationBenchmark.java | 60 +++++ .../sdk/benchmark/AllocationProfiler.java | 124 +++++++++ .../sdk/testutils/TestFlagsUtils.java | 22 +- 14 files changed, 551 insertions(+), 75 deletions(-) create mode 100644 benchmark.txt create mode 100644 src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java create mode 100644 src/test/java/dev/openfeature/sdk/benchmark/AllocationProfiler.java diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a62af64c6..a5c05c305 100644 --- a/CONTRIBUTING.md +++ b/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). diff --git a/benchmark.txt b/benchmark.txt new file mode 100644 index 000000000..696ffa24c --- /dev/null +++ b/benchmark.txt @@ -0,0 +1,239 @@ +[INFO] Scanning for projects... +[INFO] +[INFO] ------------------------< dev.openfeature:sdk >------------------------- +[INFO] Building OpenFeature Java SDK 1.12.0 +[INFO] from pom.xml +[INFO] --------------------------------[ jar ]--------------------------------- +[WARNING] Parameter 'encoding' is unknown for plugin 'maven-checkstyle-plugin:3.5.0:check (validate)' +[INFO] +[INFO] >>> jmh:0.2.2:benchmark (default-cli) > process-test-resources @ sdk >>> +[INFO] +[INFO] --- checkstyle:3.5.0:check (validate) @ sdk --- +[INFO] Starting audit... +Audit done. +[INFO] You have 0 Checkstyle violations. +[INFO] +[INFO] --- jacoco:0.8.12:prepare-agent (prepare-agent) @ sdk --- +[INFO] surefireArgLine set to -javaagent:/home/todd/.m2/repository/org/jacoco/org.jacoco.agent/0.8.12/org.jacoco.agent-0.8.12-runtime.jar=destfile=/home/todd/git/java-sdk/target/coverage-reports/jacoco-ut.exec +[INFO] +[INFO] --- resources:3.3.1:resources (default-resources) @ sdk --- +[INFO] skip non existing resourceDirectory /home/todd/git/java-sdk/src/main/resources +[INFO] +[INFO] --- compiler:3.13.0:compile (default-compile) @ sdk --- +[INFO] Recompiling the module because of changed source code. +[INFO] Compiling 65 source files with javac [debug target 1.8] to target/classes +[WARNING] bootstrap class path not set in conjunction with -source 8 +[WARNING] source value 8 is obsolete and will be removed in a future release +[WARNING] target value 8 is obsolete and will be removed in a future release +[WARNING] To suppress warnings about obsolete options, use -Xlint:-options. +[INFO] Annotation processing is enabled because one or more processors were found + on the class path. A future release of javac may disable annotation processing + unless at least one processor is specified by name (-processor), or a search + path is specified (--processor-path, --processor-module-path), or annotation + processing is enabled explicitly (-proc:only, -proc:full). + Use -Xlint:-options to suppress this message. + Use -proc:none to disable annotation processing. +[WARNING] /home/todd/git/java-sdk/src/main/java/dev/openfeature/sdk/MutableStructure.java:[19,1] Generating equals/hashCode implementation but without a call to superclass, even though this class does not extend java.lang.Object. If this is intentional, add '@EqualsAndHashCode(callSuper=false)' to your type. +[WARNING] /home/todd/git/java-sdk/src/main/java/dev/openfeature/sdk/ImmutableStructure.java:[22,1] Generating equals/hashCode implementation but without a call to superclass, even though this class does not extend java.lang.Object. If this is intentional, add '@EqualsAndHashCode(callSuper=false)' to your type. +[WARNING] /home/todd/git/java-sdk/src/main/java/dev/openfeature/sdk/EventDetails.java:[9,1] Generating equals/hashCode implementation but without a call to superclass, even though this class does not extend java.lang.Object. If this is intentional, add '@EqualsAndHashCode(callSuper=false)' to your type. +[WARNING] /home/todd/git/java-sdk/src/main/java/dev/openfeature/sdk/Value.java:[27,26] finalize() in java.lang.Object has been deprecated and marked for removal +[INFO] /home/todd/git/java-sdk/src/main/java/dev/openfeature/sdk/NoOpProvider.java: Some input files use or override a deprecated API. +[INFO] /home/todd/git/java-sdk/src/main/java/dev/openfeature/sdk/NoOpProvider.java: Recompile with -Xlint:deprecation for details. +[INFO] /home/todd/git/java-sdk/src/main/java/dev/openfeature/sdk/Value.java: Some input files use unchecked or unsafe operations. +[INFO] /home/todd/git/java-sdk/src/main/java/dev/openfeature/sdk/Value.java: Recompile with -Xlint:unchecked for details. +[INFO] +[INFO] --- resources:3.3.1:testResources (default-testResources) @ sdk --- +[INFO] Copying 2 resources from src/test/resources to target/test-classes +[INFO] +[INFO] <<< jmh:0.2.2:benchmark (default-cli) < process-test-resources @ sdk <<< +[INFO] +[INFO] +[INFO] --- jmh:0.2.2:benchmark (default-cli) @ sdk --- +[INFO] Changes detected - recompiling the module! +[INFO] Compiling 52 source files to /home/todd/git/java-sdk/target/test-classes +[INFO] /home/todd/git/java-sdk/src/test/java/dev/openfeature/sdk/LockingTest.java: Some input files use or override a deprecated API. +[INFO] /home/todd/git/java-sdk/src/test/java/dev/openfeature/sdk/LockingTest.java: Recompile with -Xlint:deprecation for details. +[INFO] /home/todd/git/java-sdk/src/test/java/dev/openfeature/sdk/internal/TriConsumerTest.java: Some input files use unchecked or unsafe operations. +[INFO] /home/todd/git/java-sdk/src/test/java/dev/openfeature/sdk/internal/TriConsumerTest.java: Recompile with -Xlint:unchecked for details. +[INFO] Executing the JMH benchmarks +# JMH version: 1.37 +# VM version: JDK 21.0.4, OpenJDK 64-Bit Server VM, 21.0.4+7 +# VM invoker: /usr/lib/jvm/java-21-openjdk/bin/java +# VM options: -Xmx1024m -XX:+UnlockExperimentalVMOptions -XX:+UseEpsilonGC -Xmx1024m -XX:+UnlockExperimentalVMOptions -XX:+UseEpsilonGC +# Blackhole mode: compiler (auto-detected, use -Djmh.blackhole.autoDetect=false to disable) +# Warmup: +# Measurement: 1 iterations, single-shot each +# Timeout: 10 min per iteration +# Threads: 1 thread +# Benchmark mode: Single shot invocation time +# Benchmark: dev.openfeature.sdk.benchmark.AllocationBenchmark.run + +# Run progress: 0.00% complete, ETA 00:00:00 +# Fork: 1 of 1 +[0.001s][warning][gc,init] Consider setting -Xms equal to -Xmx to avoid resizing hiccups +[0.001s][warning][gc,init] Consider enabling -XX:+AlwaysPreTouch to avoid memory commit hiccups +Iteration 1: num #instances #bytes class name (module) +------------------------------------------------------- + 1: 1146984 55055232 java.util.HashMap (java.base@21.0.4) + 2: 700056 11200896 java.util.HashMap$EntrySet (java.base@21.0.4) + 3: 47757 9295888 [B (java.base@21.0.4) + 4: 305989 8105752 [Ljava.lang.Object; (java.base@21.0.4) + 5: 482225 7715600 dev.openfeature.sdk.ImmutableStructure + 6: 472225 7555600 dev.openfeature.sdk.ImmutableContext + 7: 100000 4000000 dev.openfeature.sdk.HookContext + 8: 100000 4000000 dev.openfeature.sdk.HookContext$HookContextBuilder + 9: 154 2995712 [Ljdk.internal.vm.FillerElement; (java.base@21.0.4) + 10: 122807 2947368 java.util.ArrayList (java.base@21.0.4) + 11: 50000 2000000 dev.openfeature.sdk.FlagEvaluationDetails + 12: 50000 2000000 dev.openfeature.sdk.ProviderEvaluation + 13: 50002 1600064 java.util.Collections$UnmodifiableMap (java.base@21.0.4) + 14: 100001 1600016 dev.openfeature.sdk.NoOpProvider$$Lambda/0x000074760c02fa78 + 15: 50000 1600000 [Ljava.util.List; (java.base@21.0.4) + 16: 100000 1600000 dev.openfeature.sdk.ImmutableMetadata + 17: 100000 1600000 dev.openfeature.sdk.ImmutableMetadata$ImmutableMetadataBuilder + 18: 100000 1600000 dev.openfeature.sdk.OpenFeatureClient$$Lambda/0x000074760c0821f8 + 19: 43808 1401856 java.util.ArrayList$Itr (java.base@21.0.4) + 20: 50000 1200000 dev.openfeature.sdk.FlagEvaluationOptions + 21: 56919 910704 java.util.Optional (java.base@21.0.4) + 22: 34754 834096 dev.openfeature.sdk.FlagEvaluationOptions$FlagEvaluationOptionsBuilder + 23: 4489 679248 [I (java.base@21.0.4) + 24: 26554 637296 java.lang.String (java.base@21.0.4) + 25: 12462 598176 dev.openfeature.sdk.FlagEvaluationDetails$FlagEvaluationDetailsBuilder + 26: 13748 549920 dev.openfeature.sdk.ProviderEvaluation$ProviderEvaluationBuilder + 27: 16418 394032 dev.openfeature.sdk.HookSupport$$Lambda/0x000074760c081230 + 28: 1461 390008 [J (java.base@21.0.4) + 29: 24033 384528 dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock$$Lambda/0x000074760c02eae8 + 30: 14591 350184 dev.openfeature.sdk.HookSupport$$Lambda/0x000074760c081000 + 31: 2355 288104 java.lang.Class (java.base@21.0.4) + 32: 8141 260512 java.util.HashMap$EntryIterator (java.base@21.0.4) + 33: 4610 258160 jdk.internal.org.objectweb.asm.SymbolTable$Entry (java.base@21.0.4) + 34: 10001 240024 java.lang.Double (java.base@21.0.4) + 35: 2502 180144 java.lang.reflect.Field (java.base@21.0.4) + 36: 10000 160000 dev.openfeature.sdk.Value + 37: 6004 144096 java.lang.StringBuilder (java.base@21.0.4) + 38: 179 139928 [Ljdk.internal.org.objectweb.asm.SymbolTable$Entry; (java.base@21.0.4) + 39: 3824 122368 java.util.concurrent.ConcurrentHashMap$Node (java.base@21.0.4) + 40: 48 122168 [C (java.base@21.0.4) + 41: 1440 113512 [S (java.base@21.0.4) + 42: 1201 105688 java.lang.reflect.Method (java.base@21.0.4) + 43: 3030 79616 [Ljava.lang.Class; (java.base@21.0.4) + 44: 1349 75544 jdk.internal.org.objectweb.asm.Label (java.base@21.0.4) + 45: 1550 74400 java.lang.invoke.MemberName (java.base@21.0.4) + 46: 332 74368 jdk.internal.org.objectweb.asm.MethodWriter (java.base@21.0.4) + 47: 1794 71760 java.lang.invoke.MethodType (java.base@21.0.4) + 48: 1089 69696 java.net.URL (java.base@21.0.4) + 49: 2011 64352 java.util.HashMap$Node (java.base@21.0.4) + 50: 121 50512 [Ljava.util.concurrent.ConcurrentHashMap$Node; (java.base@21.0.4) + 51: 3140 50240 jdk.internal.util.StrongReferenceKey (java.base@21.0.4) + 52: 491 49608 [Ljava.util.HashMap$Node; (java.base@21.0.4) + 53: 1057 42280 java.io.ObjectStreamField (java.base@21.0.4) + 54: 1225 39200 java.io.File (java.base@21.0.4) + 55: 779 37392 jdk.internal.org.objectweb.asm.Frame (java.base@21.0.4) + 56: 243 25272 java.util.jar.JarFile$JarFileEntry (java.base@21.0.4) + 57: 793 25224 [Ljava.lang.String; (java.base@21.0.4) + 58: 622 24880 java.lang.NoSuchFieldException (java.base@21.0.4) + 59: 571 22840 java.util.LinkedHashMap$Entry (java.base@21.0.4) + 60: 473 22704 jdk.internal.ref.CleanerImpl$PhantomCleanableRef (java.base@21.0.4) + 61: 689 22048 jdk.internal.util.WeakReferenceKey (java.base@21.0.4) + 62: 824 19776 jdk.internal.org.objectweb.asm.ByteVector (java.base@21.0.4) + 63: 248 18848 [Ljava.lang.ref.SoftReference; (java.base@21.0.4) + 64: 117 17784 jdk.internal.org.objectweb.asm.ClassWriter (java.base@21.0.4) + 65: 380 16824 [Ljava.lang.invoke.LambdaForm$Name; (java.base@21.0.4) + 66: 625 15000 java.lang.Long (java.base@21.0.4) + 67: 463 14816 java.lang.invoke.LambdaForm$Name (java.base@21.0.4) + 68: 903 14448 java.lang.Object (java.base@21.0.4) + 69: 198 14256 java.lang.reflect.Constructor (java.base@21.0.4) + 70: 249 13944 java.util.zip.ZipFile$ZipFileInputStream (java.base@21.0.4) + 71: 334 13360 jdk.internal.org.objectweb.asm.Handler (java.base@21.0.4) + 72: 202 12928 java.util.concurrent.ConcurrentHashMap (java.base@21.0.4) + 73: 201 12864 jdk.internal.org.objectweb.asm.FieldWriter (java.base@21.0.4) + 74: 316 12640 java.util.WeakHashMap$Entry (java.base@21.0.4) + 75: 102 12240 java.io.ObjectStreamClass (java.base@21.0.4) + 76: 249 11952 java.util.zip.ZipFile$ZipFileInflaterInputStream (java.base@21.0.4) + 77: 359 11488 jdk.internal.org.objectweb.asm.Type (java.base@21.0.4) + 78: 464 11136 jdk.internal.org.objectweb.asm.Edge (java.base@21.0.4) + 79: 463 11112 java.lang.invoke.ResolvedMethodName (java.base@21.0.4) + 80: 341 10912 jdk.internal.math.FDBigInteger (java.base@21.0.4) + 81: 94 10728 [Ljava.lang.reflect.Field; (java.base@21.0.4) + 82: 266 10640 java.lang.NoSuchMethodException (java.base@21.0.4) + 83: 266 10640 java.security.CodeSource (java.base@21.0.4) + 84: 264 10560 sun.security.util.KnownOIDs (java.base@21.0.4) + 85: 218 10464 java.lang.invoke.DirectMethodHandle$Constructor (java.base@21.0.4) + 86: 75 10200 sun.nio.fs.UnixFileAttributes (java.base@21.0.4) + 87: 123 9840 jdk.internal.event.DeserializationEvent (java.base@21.0.4) + 88: 245 9800 java.lang.ref.SoftReference (java.base@21.0.4) + 89: 115 9200 [Ljava.util.WeakHashMap$Entry; (java.base@21.0.4) + 90: 368 8832 java.lang.module.ModuleDescriptor$Exports (java.base@21.0.4) + 91: 63 8384 [Ljava.lang.invoke.MethodHandle; (java.base@21.0.4) + 92: 146 8176 java.io.FileCleanable (java.base@21.0.4) + 93: 125 8000 java.lang.Class$ReflectionData (java.base@21.0.4) + 94: 322 7728 java.util.ImmutableCollections$Set12 (java.base@21.0.4) + 95: 120 7680 jdk.internal.org.objectweb.asm.SymbolTable (java.base@21.0.4) + 96: 69 7176 java.lang.invoke.InnerClassLambdaMetafactory (java.base@21.0.4) + 97: 144 6912 jdk.internal.org.objectweb.asm.AnnotationWriter (java.base@21.0.4) + 98: 167 6680 jdk.internal.loader.URLClassPath$JarLoader$2 (java.base@21.0.4) + 99: 196 6272 java.lang.invoke.MethodHandles$Lookup (java.base@21.0.4) + 100: 156 6240 java.util.StringJoiner (java.base@21.0.4) + 101: 153 6120 java.io.FileDescriptor (java.base@21.0.4) + 102: 126 6048 java.lang.invoke.LambdaForm (java.base@21.0.4) + 103: 77 6016 [Ljava.lang.reflect.Method; (java.base@21.0.4) + 104: 249 5976 java.util.zip.ZipFile$InflaterCleanupAction (java.base@21.0.4) + 105: 370 5920 java.lang.Byte (java.base@21.0.4) + 106: 74 5920 java.util.zip.ZipFile$Source (java.base@21.0.4) + 107: 82 5720 [Ljava.io.ObjectStreamField; (java.base@21.0.4) + 108: 40 5640 [Ljava.lang.ClassValue$Entry; (java.base@21.0.4) + 109: 234 5616 java.util.jar.Attributes$Name (java.base@21.0.4) + 110: 174 5568 java.util.concurrent.locks.ReentrantLock$NonfairSync (java.base@21.0.4) + 111: 98 5488 java.lang.Module (java.base@21.0.4) + 112: 219 5256 java.lang.PublicMethods$MethodList (java.base@21.0.4) + 113: 65 5200 java.net.URI (java.base@21.0.4) + 114: 215 5104 [Ljdk.internal.org.objectweb.asm.Type; (java.base@21.0.4) + 115: 158 5056 java.lang.invoke.MethodTypeForm (java.base@21.0.4) + 116: 152 4864 java.nio.file.attribute.FileTime (java.base@21.0.4) + 117: 301 4816 java.util.HashSet (java.base@21.0.4) + 118: 75 4800 java.util.zip.Inflater (java.base@21.0.4) +truncated... +Total 4474389 138762960 + +0.113 s/op + +totalAllocatedBytes: 138762960.000 bytes + +totalAllocatedInstances: 4474389.000 instances + +totalHeap: 521412608.000 bytes + + + +Secondary result "dev.openfeature.sdk.benchmark.AllocationBenchmark.run:+totalAllocatedBytes": + 138762960.000 bytes + +Secondary result "dev.openfeature.sdk.benchmark.AllocationBenchmark.run:+totalAllocatedInstances": + 4474389.000 instances + +Secondary result "dev.openfeature.sdk.benchmark.AllocationBenchmark.run:+totalHeap": + 521412608.000 bytes + + +# Run complete. Total time: 00:00:00 + +REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on +why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial +experiments, perform baseline and negative tests that provide experimental control, make sure +the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts. +Do not assume the numbers tell you what you want them to tell. + +NOTE: Current JVM experimentally supports Compiler Blackholes, and they are in use. Please exercise +extra caution when trusting the results, look into the generated code to check the benchmark still +works, and factor in a small probability of new VM bugs. Additionally, while comparisons between +different JVMs are already problematic, the performance difference caused by different Blackhole +modes can be very significant. Please make sure you use the consistent Blackhole mode for comparisons. + +Benchmark Mode Cnt Score Error Units +AllocationBenchmark.run ss 0.113 s/op +AllocationBenchmark.run:+totalAllocatedBytes ss 138762960.000 bytes +AllocationBenchmark.run:+totalAllocatedInstances ss 4474389.000 instances +AllocationBenchmark.run:+totalHeap ss 521412608.000 bytes +[INFO] ------------------------------------------------------------------------ +[INFO] BUILD SUCCESS +[INFO] ------------------------------------------------------------------------ +[INFO] Total time: 8.073 s +[INFO] Finished at: 2024-10-10T12:26:18-04:00 +[INFO] ------------------------------------------------------------------------ diff --git a/pom.xml b/pom.xml index 335922bee..4b9f3836d 100644 --- a/pom.xml +++ b/pom.xml @@ -1,5 +1,5 @@ - + 4.0.0 dev.openfeature @@ -11,7 +11,7 @@ 1.8 ${maven.compiler.source} 5.11.2 - + **/e2e/*.java ${project.groupId}.${project.artifactId} @@ -146,6 +146,13 @@ test + + org.openjdk.jmh + jmh-core + 1.37 + test + + @@ -473,7 +480,7 @@ 3.10.1 true - all,-missing + all,-missing @@ -507,6 +514,19 @@ + + benchmark + + + + pw.krejci + jmh-maven-plugin + 0.2.2 + + + + + e2e diff --git a/src/main/java/dev/openfeature/sdk/AbstractStructure.java b/src/main/java/dev/openfeature/sdk/AbstractStructure.java index e50fbe920..13a6cf6cb 100644 --- a/src/main/java/dev/openfeature/sdk/AbstractStructure.java +++ b/src/main/java/dev/openfeature/sdk/AbstractStructure.java @@ -8,6 +8,11 @@ abstract class AbstractStructure implements Structure { protected final Map attributes; + @Override + public boolean isEmpty() { + return attributes == null || attributes.size() == 0; + } + AbstractStructure() { this.attributes = new HashMap<>(); } @@ -32,4 +37,5 @@ public Map asObjectMap() { (accumulated, entry) -> accumulated.put(entry.getKey(), convertValue(entry.getValue())), HashMap::putAll); } + } diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index 52c5b9727..f0216b255 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/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 hooks, Map hints) { - Stream 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 void executeHooks( String hookMethod, Consumer> 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 void executeHooksUnchecked( FlagValueType flagValueType, List hooks, Consumer> 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 callBeforeHooks(FlagValueType flagValueType, HookContext hookCtx, + private EvaluationContext callBeforeHooks(FlagValueType flagValueType, HookContext hookCtx, List hooks, Map hints) { // These traverse backwards from normal. - List 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 reversedHooks = new ArrayList<>(hooks); + Collections.reverse(reversedHooks); + EvaluationContext context = hookCtx.getCtx(); + for (Hook hook : reversedHooks) { + if (hook.supportsFlagValueType(flagValueType)) { + Optional optional = Optional.ofNullable(hook.before(hookCtx, hints)) + .orElse(Optional.empty()); + if (optional.isPresent()) { + context = context.merge(optional.get()); + } + } + } + return context; } } diff --git a/src/main/java/dev/openfeature/sdk/ImmutableContext.java b/src/main/java/dev/openfeature/sdk/ImmutableContext.java index fd2ff2a68..9b27cdd59 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableContext.java +++ b/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())); diff --git a/src/main/java/dev/openfeature/sdk/ImmutableStructure.java b/src/main/java/dev/openfeature/sdk/ImmutableStructure.java index d70a01637..170602000 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableStructure.java +++ b/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 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 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 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 copyAttributes(Map in) { + Map copy = new HashMap<>(); + for (Entry entry : in.entrySet()) { + copy.put(entry.getKey(), + Optional.ofNullable(entry.getValue()).map((Value val) -> val.clone()).orElse(null)); + } + return copy; + } + } diff --git a/src/main/java/dev/openfeature/sdk/MutableContext.java b/src/main/java/dev/openfeature/sdk/MutableContext.java index 653441d31..6a47c83ef 100644 --- a/src/main/java/dev/openfeature/sdk/MutableContext.java +++ b/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 merged = this.merge( diff --git a/src/main/java/dev/openfeature/sdk/MutableStructure.java b/src/main/java/dev/openfeature/sdk/MutableStructure.java index fadd68051..1246aa5ef 100644 --- a/src/main/java/dev/openfeature/sdk/MutableStructure.java +++ b/src/main/java/dev/openfeature/sdk/MutableStructure.java @@ -30,13 +30,13 @@ public MutableStructure(Map attributes) { @Override public Set 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) { */ @Override public Map asMap() { - return new HashMap<>(this.attributes); + return new HashMap<>(attributes); } } diff --git a/src/main/java/dev/openfeature/sdk/Structure.java b/src/main/java/dev/openfeature/sdk/Structure.java index f3768e958..02e36629e 100644 --- a/src/main/java/dev/openfeature/sdk/Structure.java +++ b/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 Map merge(Function, Structure> newStructure, Map base, Map overriding) { - + + if (base.isEmpty()) { + return overriding; + } + if (overriding.isEmpty()) { + return base; + } + final Map merged = new HashMap<>(base); for (Entry overridingEntry : overriding.entrySet()) { String key = overridingEntry.getKey(); diff --git a/src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java b/src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java index 34caadaea..9e5dcf613 100644 --- a/src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java +++ b/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 defaultIfNull(T source, Supplier defaultValue) { */ @SafeVarargs public static List merge(List... sources) { - return Arrays - .stream(sources) - .flatMap(Collection::stream) - .collect(Collectors.toList()); + List merged = new ArrayList<>(); + for (List source : sources) { + merged.addAll(source); + } + return merged; } } diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java new file mode 100644 index 000000000..e6f63a98d --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java @@ -0,0 +1,60 @@ +package dev.openfeature.sdk.benchmark; + +import static dev.openfeature.sdk.testutils.TestFlagsUtils.BOOLEAN_FLAG_KEY; +import static dev.openfeature.sdk.testutils.TestFlagsUtils.FLOAT_FLAG_KEY; +import static dev.openfeature.sdk.testutils.TestFlagsUtils.INT_FLAG_KEY; +import static dev.openfeature.sdk.testutils.TestFlagsUtils.OBJECT_FLAG_KEY; +import static dev.openfeature.sdk.testutils.TestFlagsUtils.STRING_FLAG_KEY; + +import java.util.Map; +import java.util.Optional; + +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.Warmup; + +import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.Hook; +import dev.openfeature.sdk.HookContext; +import dev.openfeature.sdk.ImmutableContext; +import dev.openfeature.sdk.ImmutableStructure; +import dev.openfeature.sdk.NoOpProvider; +import dev.openfeature.sdk.OpenFeatureAPI; +import dev.openfeature.sdk.Value; + +/** + * Runs a large volume of flag evaluations on a VM with 1G memory and GC + * completely disabled so we can take a heap-dump. + */ +public class AllocationBenchmark { + + // 10K iterations works well with Xmx1024m (we don't want to run out of memory) + private static final int ITERATIONS = 10000; + + @Benchmark + @BenchmarkMode(Mode.SingleShotTime) + @Fork(jvmArgsAppend = { "-Xmx1024m", "-XX:+UnlockExperimentalVMOptions", "-XX:+UseEpsilonGC" }) + public void run() { + + OpenFeatureAPI.getInstance().setProviderAndWait(new NoOpProvider()); + Client client = OpenFeatureAPI.getInstance().getClient(); + client.addHooks(new Hook() { + @Override + public Optional before(HookContext ctx, Map hints) { + return Optional.ofNullable(new ImmutableContext()); + } + }); + + for (int i = 0; i < ITERATIONS; i++) { + client.getBooleanValue(BOOLEAN_FLAG_KEY, false); + client.getStringValue(STRING_FLAG_KEY, "default"); + client.getIntegerValue(INT_FLAG_KEY, 0); + client.getDoubleValue(FLOAT_FLAG_KEY, 0.0); + client.getObjectDetails(OBJECT_FLAG_KEY, new Value(new ImmutableStructure()), new ImmutableContext()); + } + } +} diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationProfiler.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationProfiler.java new file mode 100644 index 000000000..8051a167e --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationProfiler.java @@ -0,0 +1,124 @@ +package dev.openfeature.sdk.benchmark; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.LineNumberReader; +import java.io.PrintStream; +import java.util.ArrayList; +import java.util.Collection; + +import org.openjdk.jmh.infra.BenchmarkParams; +import org.openjdk.jmh.infra.IterationParams; +import org.openjdk.jmh.profile.InternalProfiler; +import org.openjdk.jmh.results.AggregationPolicy; +import org.openjdk.jmh.results.IterationResult; +import org.openjdk.jmh.results.Result; +import org.openjdk.jmh.results.ScalarResult; +import org.openjdk.jmh.util.Utils; + +/** + * Takes a heap dump (using JMAP from a separate process) after a benchmark; + * only useful if GC is disabled during the benchmark. + */ +public class AllocationProfiler implements InternalProfiler { + + public static class AllocationTotals { + long instances; + long bytes; + + public AllocationTotals(long instances, long bytes) { + this.instances = instances; + this.bytes = bytes; + } + } + + @Override + public String getDescription() { + return "Max memory heap profiler"; + } + + @Override + public void beforeIteration(BenchmarkParams benchmarkParams, IterationParams iterationParams) { + // intentionally left blank + } + + @Override + public Collection afterIteration(BenchmarkParams benchmarkParams, IterationParams iterationParams, + IterationResult result) { + + long totalHeap = Runtime.getRuntime().totalMemory(); + AllocationTotals allocationTotals = AllocationProfiler.printHeapHistogram(System.out, 120); + + Collection results = new ArrayList<>(); + results.add(new ScalarResult("+totalHeap", totalHeap, "bytes", AggregationPolicy.MAX)); + results.add(new ScalarResult("+totalAllocatedInstances", allocationTotals.instances, "instances", + AggregationPolicy.MAX)); + results.add(new ScalarResult("+totalAllocatedBytes", allocationTotals.bytes, "bytes", AggregationPolicy.MAX)); + + return results; + } + + private static String getJmapExcutable() { + String javaHome = System.getProperty("java.home"); + String jreDir = File.separator + "jre"; + if (javaHome.endsWith(jreDir)) { + javaHome = javaHome.substring(0, javaHome.length() - jreDir.length()); + } + return (javaHome + + File.separator + + "bin" + + File.separator + + "jmap" + + (Utils.isWindows() ? ".exe" : "")); + } + + // runs JMAP executable in a new process to collect a heap dump + // heavily inspired by: https://github.com/cache2k/cache2k-benchmark/blob/master/jmh-suite/src/main/java/org/cache2k/benchmark/jmh/HeapProfiler.java + private static AllocationTotals printHeapHistogram(PrintStream out, int maxLines) { + long totalBytes = 0; + long totalInstances = 0; + boolean partial = false; + try { + Process jmapProcess = Runtime.getRuntime().exec(new String[] { + getJmapExcutable(), + "-histo:live", + Long.toString(Utils.getPid()) }); + InputStream in = jmapProcess.getInputStream(); + LineNumberReader r = new LineNumberReader(new InputStreamReader(in)); + String line; + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + PrintStream printStream = new PrintStream(buffer); + while ((line = r.readLine()) != null) { + if (line.startsWith("Total")) { + printStream.println(line); + String[] tokens = line.split("\\s+"); + totalInstances += Long.parseLong(tokens[1]); + totalBytes = Long.parseLong(tokens[2]); + } else if (r.getLineNumber() <= maxLines) { + printStream.println(line); + } else { + if (!partial) { + printStream.println("truncated..."); + } + partial = true; + } + } + r.close(); + in.close(); + printStream.close(); + byte[] histogramOutput = buffer.toByteArray(); + buffer = new ByteArrayOutputStream(); + printStream = new PrintStream(buffer); + printStream.write(histogramOutput); + printStream.println(); + printStream.close(); + out.write(buffer.toByteArray()); + } catch (Exception ex) { + System.err.println("ForcedGcMemoryProfiler: error attaching / reading histogram"); + ex.printStackTrace(); + } + return new AllocationTotals(totalInstances, totalBytes); + } +} \ No newline at end of file diff --git a/src/test/java/dev/openfeature/sdk/testutils/TestFlagsUtils.java b/src/test/java/dev/openfeature/sdk/testutils/TestFlagsUtils.java index d90359294..dd2d03ca1 100644 --- a/src/test/java/dev/openfeature/sdk/testutils/TestFlagsUtils.java +++ b/src/test/java/dev/openfeature/sdk/testutils/TestFlagsUtils.java @@ -16,33 +16,41 @@ @UtilityClass public class TestFlagsUtils { + public static final String BOOLEAN_FLAG_KEY = "boolean-flag"; + public static final String STRING_FLAG_KEY = "string-flag"; + public static final String INT_FLAG_KEY = "integer-flag"; + public static final String FLOAT_FLAG_KEY = "float-flag"; + public static final String OBJECT_FLAG_KEY = "object-flag"; + public static final String CONTEXT_AWARE_FLAG_KEY = "context-aware"; + public static final String WRONG_FLAG_KEY = "wrong-flag"; + /** * Building flags for testing purposes. * @return map of flags */ public static Map> buildFlags() { Map> flags = new HashMap<>(); - flags.put("boolean-flag", Flag.builder() + flags.put(BOOLEAN_FLAG_KEY, Flag.builder() .variant("on", true) .variant("off", false) .defaultVariant("on") .build()); - flags.put("string-flag", Flag.builder() + flags.put(STRING_FLAG_KEY, Flag.builder() .variant("greeting", "hi") .variant("parting", "bye") .defaultVariant("greeting") .build()); - flags.put("integer-flag", Flag.builder() + flags.put(INT_FLAG_KEY, Flag.builder() .variant("one", 1) .variant("ten", 10) .defaultVariant("ten") .build()); - flags.put("float-flag", Flag.builder() + flags.put(FLOAT_FLAG_KEY, Flag.builder() .variant("tenth", 0.1) .variant("half", 0.5) .defaultVariant("half") .build()); - flags.put("object-flag", Flag.builder() + flags.put(OBJECT_FLAG_KEY, Flag.builder() .variant("empty", new HashMap<>()) .variant("template", new Value(mapToStructure(ImmutableMap.of( "showImages", new Value(true), @@ -51,7 +59,7 @@ public static Map> buildFlags() { )))) .defaultVariant("template") .build()); - flags.put("context-aware", Flag.builder() + flags.put(CONTEXT_AWARE_FLAG_KEY, Flag.builder() .variant("internal", "INTERNAL") .variant("external", "EXTERNAL") .defaultVariant("external") @@ -63,7 +71,7 @@ public static Map> buildFlags() { } }) .build()); - flags.put("wrong-flag", Flag.builder() + flags.put(WRONG_FLAG_KEY, Flag.builder() .variant("one", "uno") .variant("two", "dos") .defaultVariant("one")