From c8b1c33e773a4fd3b222d22eb2f1e9c9b196c8b2 Mon Sep 17 00:00:00 2001 From: Elliot Jackson <13633636+elliotmjackson@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:37:36 -0400 Subject: [PATCH] perf: cache deserialized feature definitions in FeatureEvaluator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deserializing a feature definition dominates evaluation cost. Profiling a caller that evaluates ~50 features per request put 44% of process CPU in evaluateFeature, three quarters of it inside Gson.fromJson — more than every database query on the endpoint combined. The parsed Feature depends only on the feature JSON, never on the context or attributes, so it is re-derived for nothing. Callers that build a GrowthBook per evaluation — the normal shape when the context carries per-request attributes — pay it on every single call. Cache the parsed definition, keyed by feature name and scoped to one features document identified by reference. A new features object (an API refresh, or setFeatures) replaces the map wholesale. Reference identity is deliberate: JsonObject.equals walks the whole tree and would cost as much as the parse it avoids. The cache is static because a FeatureEvaluator is created per GrowthBook, so a per-instance cache would never be reused by those callers. Entries are safe to share: Feature exposes only final fields and nothing in evaluation mutates a feature or its rules. Measured on the calling service, API pinned to 2 cores to remove load-generator contention: 104 -> 220 rps at 100% success, median latency 17ms -> 9.9ms. Roughly double the throughput per core. The existing conformance suite passes unchanged (132 tests), plus 4 new tests covering cache scoping and invalidation. --- .../growthbook/sdk/java/FeatureEvaluator.java | 61 ++++++++++++- ...eatureEvaluatorParsedFeatureCacheTest.java | 90 +++++++++++++++++++ 2 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 lib/src/test/java/growthbook/sdk/java/FeatureEvaluatorParsedFeatureCacheTest.java diff --git a/lib/src/main/java/growthbook/sdk/java/FeatureEvaluator.java b/lib/src/main/java/growthbook/sdk/java/FeatureEvaluator.java index 8e6de298..362ba5c7 100644 --- a/lib/src/main/java/growthbook/sdk/java/FeatureEvaluator.java +++ b/lib/src/main/java/growthbook/sdk/java/FeatureEvaluator.java @@ -11,6 +11,8 @@ import java.util.List; import java.util.HashSet; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicReference; /** * INTERNAL: Implementation of feature evaluation. @@ -21,6 +23,63 @@ class FeatureEvaluator implements IFeatureEvaluator { private final GrowthBookJsonUtils jsonUtils = GrowthBookJsonUtils.getInstance(); + + /** + * Deserialized feature definitions, shared across evaluators. + * + *

Deserializing a feature definition is the dominant cost of evaluation — profiling a caller + * that evaluates ~50 features per request found 44% of process CPU in {@code evaluateFeature}, + * three quarters of it inside {@code Gson.fromJson} — and the result depends only on the feature + * JSON, never on the context or attributes. Callers that build a GrowthBook instance per + * evaluation (a common pattern, since the context carries per-request attributes) otherwise + * re-parse the same definitions on every call. + * + *

The cache is static because a {@link FeatureEvaluator} is created per {@link GrowthBook}, + * so a per-instance cache would never be reused by those callers. It is keyed by feature name + * and scoped to one features document, identified by reference: when the SDK is handed a + * different features object — a refresh from the API, or a call to + * {@link GBContext#setFeatures} — the whole map is replaced rather than invalidated entry by + * entry. Reference identity is deliberate; {@code JsonObject.equals} would walk the entire tree + * and cost as much as the parse it is meant to avoid. + * + *

Entries are safe to share: {@link Feature} exposes only final fields and nothing in + * evaluation mutates a feature or its rules. + */ + private static final AtomicReference PARSED_FEATURES = new AtomicReference<>(); + + private static final class ParsedFeatures { + private final JsonObject source; + private final ConcurrentHashMap> byKey = new ConcurrentHashMap<>(); + + private ParsedFeatures(JsonObject source) { + this.source = source; + } + } + + /** + * Returns the deserialized definition for {@code key}, parsing it only the first time it is seen + * for this features document. + */ + @SuppressWarnings("unchecked") + private Feature parseFeature( + JsonObject featuresJson, + String key, + JsonElement featureJson + ) { + ParsedFeatures parsed = PARSED_FEATURES.get(); + if (parsed == null || parsed.source != featuresJson) { + parsed = new ParsedFeatures(featuresJson); + PARSED_FEATURES.set(parsed); + } + + // computeIfAbsent stores nothing when the mapper returns null, so an unparseable definition + // falls through to the caller's existing null handling instead of being memoized. + return (Feature) parsed.byKey.computeIfAbsent( + key, + unused -> jsonUtils.gson.fromJson(featureJson, Feature.class) + ); + } + private final ConditionEvaluator conditionEvaluator = new ConditionEvaluator(); private final ExperimentEvaluator experimentEvaluator = new ExperimentEvaluator(); private final FeatureEvalContext featureEvalContext = new FeatureEvalContext(null, new HashSet<>()); @@ -112,7 +171,7 @@ public FeatureResult evaluateFeature( return defaultValueFeature; } - Feature feature = jsonUtils.gson.fromJson(featureJson, Feature.class); + Feature feature = parseFeature(featuresJson, key, featureJson); if (feature == null) { // When key exists but there is no value, should be default value with null value if (featureUsageCallback != null) { diff --git a/lib/src/test/java/growthbook/sdk/java/FeatureEvaluatorParsedFeatureCacheTest.java b/lib/src/test/java/growthbook/sdk/java/FeatureEvaluatorParsedFeatureCacheTest.java new file mode 100644 index 00000000..a428428f --- /dev/null +++ b/lib/src/test/java/growthbook/sdk/java/FeatureEvaluatorParsedFeatureCacheTest.java @@ -0,0 +1,90 @@ +package growthbook.sdk.java; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import com.google.gson.JsonObject; +import org.junit.jupiter.api.Test; + +/** + * Covers the deserialized-feature cache in {@link FeatureEvaluator}. + * + *

Deserializing a feature definition dominates evaluation cost, and the result depends only on + * the feature JSON. The cache exists so that a caller building a GrowthBook per evaluation — which + * is what you do when the context carries per-request attributes — does not re-parse the same + * definitions on every call. These tests pin the two properties that make it safe: a changed + * features document must not be served from the old cache, and caching must not change what + * evaluation returns. + */ +class FeatureEvaluatorParsedFeatureCacheTest { + + private static final String FEATURE_KEY = "cached_feature"; + + private GBContext contextReturning(String defaultValue) { + return GBContext + .builder() + .featuresJson("{\"" + FEATURE_KEY + "\":{\"defaultValue\":\"" + defaultValue + "\"}}") + .attributesJson("{\"id\":\"user-1\"}") + .build(); + } + + @Test + void repeatedEvaluationsAgainstOneFeaturesDocumentReturnTheSameValue() { + FeatureEvaluator evaluator = new FeatureEvaluator(); + GBContext context = contextReturning("hello"); + + for (int i = 0; i < 5; i++) { + FeatureResult result = + evaluator.evaluateFeature(FEATURE_KEY, context, String.class, new JsonObject()); + assertNotNull(result); + assertEquals("hello", result.getValue()); + } + } + + /** + * The cache is scoped to a features document by reference, so replacing the document has to + * replace the cache. If it did not, a feature refresh would keep serving stale definitions for + * the lifetime of the process — the worst possible failure for a feature flag system. + */ + @Test + void replacingTheFeaturesDocumentReplacesTheCachedDefinition() { + FeatureEvaluator evaluator = new FeatureEvaluator(); + + FeatureResult before = evaluator.evaluateFeature( + FEATURE_KEY, contextReturning("old"), String.class, new JsonObject()); + FeatureResult after = evaluator.evaluateFeature( + FEATURE_KEY, contextReturning("new"), String.class, new JsonObject()); + + assertEquals("old", before.getValue()); + assertEquals("new", after.getValue()); + } + + /** + * Callers that construct a GrowthBook per evaluation get a fresh FeatureEvaluator each time, + * which is exactly the case the cache is static for. + */ + @Test + void separateEvaluatorsShareTheCacheWithoutAffectingResults() { + GBContext context = contextReturning("shared"); + + FeatureResult first = new FeatureEvaluator() + .evaluateFeature(FEATURE_KEY, context, String.class, new JsonObject()); + FeatureResult second = new FeatureEvaluator() + .evaluateFeature(FEATURE_KEY, context, String.class, new JsonObject()); + + assertEquals("shared", first.getValue()); + assertEquals("shared", second.getValue()); + } + + @Test + void anUnknownKeyIsStillUnknownAfterAnotherKeyHasBeenCached() { + FeatureEvaluator evaluator = new FeatureEvaluator(); + GBContext context = contextReturning("hello"); + + evaluator.evaluateFeature(FEATURE_KEY, context, String.class, new JsonObject()); + FeatureResult unknown = + evaluator.evaluateFeature("no_such_feature", context, String.class, new JsonObject()); + + assertEquals(FeatureResultSource.UNKNOWN_FEATURE, unknown.getSource()); + } +}