From b38ea137e563a9ebf84835ed7e5e2a39ac9809e2 Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Thu, 16 Jul 2026 14:53:15 +0200 Subject: [PATCH] Improve interpreter lookup paths Cache protobuf field metadata lookups per registry and avoid duplicate allocation under concurrent access. Reduce map activation lookups for non-null bindings and use checked overload IDs for custom function dispatch. --- .../common/types/pb/ProtoTypeRegistry.java | 8 ++++++ .../cel/interpreter/Activation.java | 7 +++--- .../cel/interpreter/InterpretablePlanner.java | 2 +- .../java/org/projectnessie/cel/CELTest.java | 25 +++++++++++++++++++ .../cel/interpreter/ActivationTest.java | 10 ++++++++ 5 files changed, 47 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java b/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java index 3d03d9bb9..5499447d8 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/pb/ProtoTypeRegistry.java @@ -78,6 +78,7 @@ import java.util.Map.Entry; import java.util.Objects; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import org.projectnessie.cel.common.types.TypeT; import org.projectnessie.cel.common.types.ref.FieldType; import org.projectnessie.cel.common.types.ref.TypeRegistry; @@ -87,11 +88,13 @@ public final class ProtoTypeRegistry implements TypeRegistry { private static final ProtoTypeRegistry DEFAULT_REGISTRY = newDefaultRegistry(); private final Map revTypeMap; + private final Map fieldTypeCache; private final Db pbdb; private ProtoTypeRegistry( Map revTypeMap, Db pbdb) { this.revTypeMap = revTypeMap; + this.fieldTypeCache = new ConcurrentHashMap<>(); this.pbdb = pbdb; } @@ -197,6 +200,11 @@ public Val enumValue(String enumName) { @Override public FieldType findFieldType(String messageType, String fieldName) { + String cacheKey = messageType + '\n' + fieldName; + return fieldTypeCache.computeIfAbsent(cacheKey, key -> loadFieldType(messageType, fieldName)); + } + + private FieldType loadFieldType(String messageType, String fieldName) { PbTypeDescription msgType = pbdb.describeType(messageType); if (msgType == null) { return null; diff --git a/core/src/main/java/org/projectnessie/cel/interpreter/Activation.java b/core/src/main/java/org/projectnessie/cel/interpreter/Activation.java index ab7fbcac3..ff0ea5e29 100644 --- a/core/src/main/java/org/projectnessie/cel/interpreter/Activation.java +++ b/core/src/main/java/org/projectnessie/cel/interpreter/Activation.java @@ -101,12 +101,11 @@ public Activation parent() { /** ResolveName implements the Activation interface method. */ @Override public ResolvedValue resolveName(String name) { - if (!bindings.containsKey(name)) { - return ResolvedValue.ABSENT; - } - Object obj = bindings.get(name); if (obj == null) { + if (!bindings.containsKey(name)) { + return ResolvedValue.ABSENT; + } return ResolvedValue.NULL_VALUE; } diff --git a/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java b/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java index cce8a3e8c..fcf9f712c 100644 --- a/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java +++ b/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java @@ -352,7 +352,7 @@ Interpretable planCall(Expr expr) { // Otherwise, generate Interpretable calls specialized by argument count. // Try to find the specific function by overload id. Overload fnDef = null; - if (resolvedFunc.overloadId != null && resolvedFunc.overloadId.isEmpty()) { + if (resolvedFunc.overloadId != null && !resolvedFunc.overloadId.isEmpty()) { fnDef = disp.findOverload(resolvedFunc.overloadId); } // If the overload id couldn't resolve the function, try the simple function name. diff --git a/core/src/test/java/org/projectnessie/cel/CELTest.java b/core/src/test/java/org/projectnessie/cel/CELTest.java index 15d94cfea..992ce8289 100644 --- a/core/src/test/java/org/projectnessie/cel/CELTest.java +++ b/core/src/test/java/org/projectnessie/cel/CELTest.java @@ -284,6 +284,31 @@ void CustomEnvCanSubsetStandardLibrary() { assertThat(prg.eval(mapOf("resource.name", "buckets/example")).getVal()).isSameAs(False); } + @Test + void ProgramUsesCheckedOverloadIdForCustomFunctionDispatch() { + Env env = + newEnv( + declarations( + Decls.newVar("value", Decls.Int), + Decls.newFunction( + "is_even", + Decls.newOverload("is_even_int", singletonList(Decls.Int), Decls.Bool)))); + + AstIssuesTuple astIss = env.compile("is_even(value)"); + assertThat(astIss.hasIssues()).isFalse(); + + Program program = + env.program( + astIss.getAst(), + functions( + Overload.unary( + "is_even_int", + value -> boolOf(((Number) value.value()).longValue() % 2 == 0)))); + + assertThat(program.eval(mapOf("value", 42L)).getVal()).isSameAs(True); + assertThat(program.eval(mapOf("value", 41L)).getVal()).isSameAs(False); + } + @Test void HomogeneousAggregateLiterals() { Env e = diff --git a/core/src/test/java/org/projectnessie/cel/interpreter/ActivationTest.java b/core/src/test/java/org/projectnessie/cel/interpreter/ActivationTest.java index 7a260e247..75c0a0555 100644 --- a/core/src/test/java/org/projectnessie/cel/interpreter/ActivationTest.java +++ b/core/src/test/java/org/projectnessie/cel/interpreter/ActivationTest.java @@ -53,6 +53,16 @@ void resolve() { assertThat(activation.resolveName("a").value()).isSameAs(True); } + @Test + void resolveNullAndAbsentFromMapActivation() { + Map map = new HashMap<>(); + map.put("nullValue", null); + Activation activation = newActivation(map); + + assertThat(activation.resolveName("nullValue")).isSameAs(ResolvedValue.NULL_VALUE); + assertThat(activation.resolveName("absent")).isSameAs(ResolvedValue.ABSENT); + } + @Test void resolveLazy() { AtomicReference v = new AtomicReference<>();