From 7f3f57a0b682e3639c3f3a0c22d958eb903cbdc1 Mon Sep 17 00:00:00 2001 From: Sean Huh Date: Fri, 30 Oct 2026 15:41:24 -0700 Subject: [PATCH] Avoid hotpath cast to OptimizedFunctionOverload in DynamicDispatchOverload PiperOrigin-RevId: 1010844466 --- .../dev/cel/runtime/CelResolvedOverload.java | 6 +-- .../dev/cel/runtime/DefaultDispatcher.java | 2 +- .../dev/cel/runtime/FunctionBindingImpl.java | 40 ++++++++++--------- .../runtime/OptimizedFunctionOverload.java | 6 +++ .../cel/runtime/FunctionBindingImplTest.java | 12 +++--- 5 files changed, 37 insertions(+), 29 deletions(-) diff --git a/runtime/src/main/java/dev/cel/runtime/CelResolvedOverload.java b/runtime/src/main/java/dev/cel/runtime/CelResolvedOverload.java index fbe9a3289..42ccdd031 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelResolvedOverload.java +++ b/runtime/src/main/java/dev/cel/runtime/CelResolvedOverload.java @@ -106,17 +106,13 @@ public static CelResolvedOverload of( CelFunctionOverload definition, boolean isStrict, List> parameterTypes) { - OptimizedFunctionOverload optimizedDef = - (definition instanceof OptimizedFunctionOverload) - ? (OptimizedFunctionOverload) definition - : definition::apply; return new AutoValue_CelResolvedOverload( functionName, overloadId, ImmutableList.copyOf(parameterTypes), isStrict, definition, - optimizedDef); + OptimizedFunctionOverload.of(definition)); } /** diff --git a/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java b/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java index 0a467db81..3fcc1d248 100644 --- a/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java +++ b/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java @@ -189,7 +189,7 @@ private OverloadEntry mergeDynamicDispatchesOrThrow( DynamicDispatchOverload mergedOverload = new DynamicDispatchOverload( overloadId, - ImmutableSet.builder() + ImmutableSet.builder() .addAll(existingOverload.getOverloadBindings()) .addAll(incomingOverload.getOverloadBindings()) .build()); diff --git a/runtime/src/main/java/dev/cel/runtime/FunctionBindingImpl.java b/runtime/src/main/java/dev/cel/runtime/FunctionBindingImpl.java index fb641fd8d..806b9d7dd 100644 --- a/runtime/src/main/java/dev/cel/runtime/FunctionBindingImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/FunctionBindingImpl.java @@ -33,6 +33,8 @@ final class FunctionBindingImpl implements InternalCelFunctionBinding { private final CelFunctionOverload definition; + private final OptimizedFunctionOverload optimizedDefinition; + private final boolean isStrict; @Override @@ -55,6 +57,10 @@ public CelFunctionOverload getDefinition() { return definition; } + OptimizedFunctionOverload getOptimizedDefinition() { + return optimizedDefinition; + } + @Override public boolean isStrict() { return isStrict; @@ -70,6 +76,7 @@ public boolean isStrict() { this.overloadId = overloadId; this.argTypes = argTypes; this.definition = definition; + this.optimizedDefinition = OptimizedFunctionOverload.of(definition); this.isStrict = isStrict; } @@ -83,12 +90,15 @@ public boolean isStrict() { static ImmutableSet groupOverloadsToFunction( String functionName, ImmutableSet overloadBindings) { - ImmutableSet.Builder builder = ImmutableSet.builder(); + ImmutableSet.Builder functionBindingsBuilder = ImmutableSet.builder(); for (CelFunctionBinding b : overloadBindings) { - builder.add( + functionBindingsBuilder.add( new FunctionBindingImpl( functionName, b.getOverloadId(), b.getArgTypes(), b.getDefinition(), b.isStrict())); } + ImmutableSet functionBindings = functionBindingsBuilder.build(); + ImmutableSet.Builder builder = + ImmutableSet.builder().addAll(functionBindings); // If there is already a binding with the same name as the function, we treat it as a // "Singleton" binding and do not create a dynamic dispatch wrapper for it. @@ -107,7 +117,7 @@ static ImmutableSet groupOverloadsToFunction( singleBinding.getDefinition(), singleBinding.isStrict())); } else if (overloadBindings.size() > 1) { - builder.add(new DynamicDispatchBinding(functionName, overloadBindings)); + builder.add(new DynamicDispatchBinding(functionName, functionBindings)); } } @@ -146,7 +156,7 @@ public boolean isStrict() { } private DynamicDispatchBinding( - String functionName, ImmutableSet overloadBindings) { + String functionName, ImmutableSet overloadBindings) { this.isStrict = overloadBindings.stream().allMatch(CelFunctionBinding::isStrict); this.dynamicDispatchOverload = new DynamicDispatchOverload(functionName, overloadBindings); } @@ -155,11 +165,11 @@ private DynamicDispatchBinding( @Immutable static final class DynamicDispatchOverload implements OptimizedFunctionOverload { private final String functionName; - private final ImmutableSet overloadBindings; + private final ImmutableSet overloadBindings; @Override public Object apply(Object[] args) throws CelEvaluationException { - for (CelFunctionBinding overload : overloadBindings) { + for (FunctionBindingImpl overload : overloadBindings) { if (CelFunctionOverload.canHandle(args, overload.getArgTypes(), overload.isStrict())) { return overload.getDefinition().apply(args); } @@ -174,12 +184,9 @@ public Object apply(Object[] args) throws CelEvaluationException { @Override public Object apply(Object arg) throws CelEvaluationException { - for (CelFunctionBinding overload : overloadBindings) { + for (FunctionBindingImpl overload : overloadBindings) { if (CelFunctionOverload.canHandle(arg, overload.getArgTypes(), overload.isStrict())) { - // Unary function binding setups always instantiate an OptimizedFunctionOverload, avoiding - // an instanceof check on the hotpath. - OptimizedFunctionOverload def = (OptimizedFunctionOverload) overload.getDefinition(); - return def.apply(arg); + return overload.getOptimizedDefinition().apply(arg); } } throw new CelOverloadNotFoundException( @@ -191,13 +198,10 @@ public Object apply(Object arg) throws CelEvaluationException { @Override public Object apply(Object arg1, Object arg2) throws CelEvaluationException { - for (CelFunctionBinding overload : overloadBindings) { + for (FunctionBindingImpl overload : overloadBindings) { if (CelFunctionOverload.canHandle( arg1, arg2, overload.getArgTypes(), overload.isStrict())) { - // Binary function binding setups always instantiate an OptimizedFunctionOverload, - // avoiding an instanceof check on the hotpath. - OptimizedFunctionOverload def = (OptimizedFunctionOverload) overload.getDefinition(); - return def.apply(arg1, arg2); + return overload.getOptimizedDefinition().apply(arg1, arg2); } } throw new CelOverloadNotFoundException( @@ -207,12 +211,12 @@ public Object apply(Object arg1, Object arg2) throws CelEvaluationException { .collect(toImmutableList())); } - ImmutableSet getOverloadBindings() { + ImmutableSet getOverloadBindings() { return overloadBindings; } DynamicDispatchOverload( - String functionName, ImmutableSet overloadBindings) { + String functionName, ImmutableSet overloadBindings) { this.functionName = functionName; this.overloadBindings = overloadBindings; } diff --git a/runtime/src/main/java/dev/cel/runtime/OptimizedFunctionOverload.java b/runtime/src/main/java/dev/cel/runtime/OptimizedFunctionOverload.java index fde8bcc15..f5341180d 100644 --- a/runtime/src/main/java/dev/cel/runtime/OptimizedFunctionOverload.java +++ b/runtime/src/main/java/dev/cel/runtime/OptimizedFunctionOverload.java @@ -32,4 +32,10 @@ default Object apply(Object arg) throws CelEvaluationException { default Object apply(Object arg1, Object arg2) throws CelEvaluationException { return apply(new Object[] {arg1, arg2}); } + + static OptimizedFunctionOverload of(CelFunctionOverload overload) { + return (overload instanceof OptimizedFunctionOverload) + ? (OptimizedFunctionOverload) overload + : overload::apply; + } } diff --git a/runtime/src/test/java/dev/cel/runtime/FunctionBindingImplTest.java b/runtime/src/test/java/dev/cel/runtime/FunctionBindingImplTest.java index 0395cf560..7f1a20df7 100644 --- a/runtime/src/test/java/dev/cel/runtime/FunctionBindingImplTest.java +++ b/runtime/src/test/java/dev/cel/runtime/FunctionBindingImplTest.java @@ -31,7 +31,7 @@ public final class FunctionBindingImplTest { @Test - public void dynamicDispatch_unaryOptimizedOverload_invokesOptimizedUnaryApply() throws Exception { + public void dynamicDispatch_unary_invokesOptimizedApplyAndAdaptsNonOptimized() throws Exception { OptimizedFunctionOverload mockOverload = new OptimizedFunctionOverload() { @Override @@ -48,7 +48,8 @@ public Object apply(Object[] args) { CelFunctionBinding.from( "custom_opt_unary_long", ImmutableList.of(Long.class), mockOverload); CelFunctionBinding b2 = - CelFunctionBinding.from("custom_opt_unary_str", String.class, (String arg) -> arg + "!"); + CelFunctionBinding.from( + "custom_unary_str", ImmutableList.of(String.class), args -> args[0] + "!"); ImmutableSet bindings = FunctionBindingImpl.groupOverloadsToFunction("custom_opt_unary", ImmutableSet.of(b1, b2)); OptimizedFunctionOverload overload = @@ -61,8 +62,7 @@ public Object apply(Object[] args) { } @Test - public void dynamicDispatch_binaryOptimizedOverload_invokesOptimizedBinaryApply() - throws Exception { + public void dynamicDispatch_binary_invokesOptimizedApplyAndAdaptsNonOptimized() throws Exception { OptimizedFunctionOverload mockOverload = new OptimizedFunctionOverload() { @Override @@ -81,7 +81,9 @@ public Object apply(Object[] args) { "custom_opt_binary_long", ImmutableList.of(Long.class, Long.class), mockOverload); CelFunctionBinding b2 = CelFunctionBinding.from( - "custom_opt_bin_str", String.class, String.class, (String a, String b) -> a + b); + "custom_bin_str", + ImmutableList.of(String.class, String.class), + args -> (String) args[0] + args[1]); ImmutableSet bindings = FunctionBindingImpl.groupOverloadsToFunction("custom_opt_binary", ImmutableSet.of(b1, b2)); OptimizedFunctionOverload overload =