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()); + } +}