Skip to content

Avoid O(N^2) list accumulator copies in ProgramPlanner comprehensions #1333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
copybara-service merged 1 commit into from
Oct 9, 2026
Merged

Avoid O(N^2) list accumulator copies in ProgramPlanner comprehensions #1333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ java_library(
"//common/exceptions:runtime_exception",
"//common/values",
"//runtime:accumulated_unknowns",
"//runtime:concatenated_list_view",
"//runtime:evaluation_exception",
"//runtime:interpretable",
"//runtime:interpreter_util",
Expand Down Expand Up @@ -1119,6 +1120,7 @@ cel_android_library(
"//common/exceptions:runtime_exception",
"//common/values:values_android",
"//runtime:accumulated_unknowns_android",
"//runtime:concatenated_list_view",
"//runtime:evaluation_exception",
"//runtime:interpretable_android",
"//runtime:interpreter_util_android",
Expand Down
46 changes: 22 additions & 24 deletions runtime/src/main/java/dev/cel/runtime/planner/EvalFold.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,13 @@ Object evalInternal(GlobalResolver resolver, ExecutionFrame frame) throws CelEva
}
Folder folder = new Folder(resolver, frame, accuInit, accuVar, iterVar, iterVar2);

Object result;
if (iterRangeRaw instanceof Map) {
result = evalMap((Map<?, ?>) iterRangeRaw, folder, frame);
return evalMap((Map<?, ?>) iterRangeRaw, folder, frame);
} else if (iterRangeRaw instanceof Collection) {
result = evalList((Collection<?>) iterRangeRaw, folder, frame);
return evalList((Collection<?>) iterRangeRaw, folder, frame);
} else {
throw new IllegalArgumentException("Unexpected iter_range type: " + iterRangeRaw.getClass());
}

return maybeUnwrapAccumulator(result);
}

private Object evalMap(Map<?, ?> iterRange, Folder folder, ExecutionFrame frame)
Expand All @@ -94,16 +91,14 @@ private Object evalMap(Map<?, ?> iterRange, Folder folder, ExecutionFrame frame)
}
boolean cond = (boolean) condResult;
if (!cond) {
folder.computeResult = true;
return result.eval(folder, frame);
break;
}

Object stepResult = loopStep.eval(folder, frame);
folder.accuVal = mergeAccumulator(folder.accuVal, stepResult);
folder.initialized = true;
}
folder.computeResult = true;
return result.eval(folder, frame);
return folder.evalResult(result);
}

private Object evalList(Collection<?> iterRange, Folder folder, ExecutionFrame frame)
Expand All @@ -129,17 +124,15 @@ private Object evalList(Collection<?> iterRange, Folder folder, ExecutionFrame f
}
boolean cond = (boolean) condResult;
if (!cond) {
folder.computeResult = true;
return result.eval(folder, frame);
break;
}

Object stepResult = loopStep.eval(folder, frame);
folder.accuVal = mergeAccumulator(folder.accuVal, stepResult);
folder.initialized = true;
index++;
}
folder.computeResult = true;
return result.eval(folder, frame);
return folder.evalResult(result);
}

private static Object mergeAccumulator(@Nullable Object currentAccu, Object newVal) {
Expand All @@ -159,16 +152,6 @@ private static Object maybeWrapAccumulator(Object val) {
return val;
}

private static Object maybeUnwrapAccumulator(Object val) {
if (val instanceof ConcatenatedListView) {
return ImmutableList.copyOf((ConcatenatedListView<?>) val);
}
if (val instanceof MutableMapValue) {
return ImmutableMap.copyOf((MutableMapValue) val);
}
return val;
}

private EvalFold(
CelExpr expr,
String accuVar,
Expand Down Expand Up @@ -220,7 +203,11 @@ public boolean isLocallyBound(String name) {
if (!initialized) {
initialized = true;
try {
accuVal = maybeWrapAccumulator(accuInit.eval(resolver, frame));
Object initVal = accuInit.eval(resolver, frame);
accuVal =
!computeResult && frame.enableShortCircuiting()
? maybeWrapAccumulator(initVal)
: initVal;
} catch (CelEvaluationException e) {
throw new LazyEvaluationRuntimeException(e);
}
Expand All @@ -241,6 +228,17 @@ public boolean isLocallyBound(String name) {
return resolver.resolve(name);
}

private Object evalResult(PlannedInterpretable result) throws CelEvaluationException {
computeResult = true;
// Materialize the mutable accumulator so the result expr never observes in-place mutation.
if (accuVal instanceof ConcatenatedListView) {
accuVal = ImmutableList.copyOf((ConcatenatedListView<?>) accuVal);
} else if (accuVal instanceof MutableMapValue) {
accuVal = ImmutableMap.copyOf((MutableMapValue) accuVal);
}
return result.eval(this, frame);
}

private Folder(
GlobalResolver resolver,
ExecutionFrame frame,
Expand Down
6 changes: 5 additions & 1 deletion runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import dev.cel.runtime.CelEvaluationException;
import dev.cel.runtime.CelResolvedOverload;
import dev.cel.runtime.CelUnknownSet;
import dev.cel.runtime.ConcatenatedListView;
import dev.cel.runtime.GlobalResolver;
import dev.cel.runtime.InterpreterUtil;

Expand Down Expand Up @@ -130,7 +131,10 @@ static Object maybeAdaptNonStrictArg(Object val) {
* adapts any public {@link CelUnknownSet} instances into internal {@link AccumulatedUnknowns} for
* AST evaluation.
*/
private static Object convertAndAdaptResult(CelValueConverter valueConverter, Object result) {
static Object convertAndAdaptResult(CelValueConverter valueConverter, Object result) {
if (result instanceof ConcatenatedListView) {
return result;
}
return InterpreterUtil.maybeAdaptToAccumulatedUnknowns(
valueConverter.maybeUnwrap(valueConverter.toRuntimeValue(result)));
}
Expand Down
23 changes: 10 additions & 13 deletions runtime/src/main/java/dev/cel/runtime/planner/ExecutionFrame.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
final class ExecutionFrame {

private final int comprehensionIterationLimit;
private final boolean enableShortCircuiting;
private final CelFunctionResolver functionResolver;
private final @Nullable PartialVars partialVars;
private final @Nullable CelEvaluationListener listener;
Expand All @@ -45,11 +46,7 @@ static ExecutionFrame create(
@Nullable PartialVars partialVars,
@Nullable CelEvaluationListener listener) {
return new ExecutionFrame(
functionResolver,
getComprehensionMaxIterations(celOptions),
partialVars,
listener,
/* asyncTracker= */ null);
functionResolver, celOptions, partialVars, listener, /* asyncTracker= */ null);
}

static ExecutionFrame createForAsync(
Expand All @@ -59,12 +56,7 @@ static ExecutionFrame createForAsync(
@Nullable CelEvaluationListener listener,
AsyncCallStateTracker asyncTracker) {
checkNotNull(asyncTracker, "asyncTracker");
return new ExecutionFrame(
functionResolver,
getComprehensionMaxIterations(celOptions),
partialVars,
listener,
asyncTracker);
return new ExecutionFrame(functionResolver, celOptions, partialVars, listener, asyncTracker);
}

private static int getComprehensionMaxIterations(CelOptions celOptions) {
Expand Down Expand Up @@ -109,6 +101,10 @@ AsyncCallStateTracker asyncTracker() {
return asyncTracker;
}

boolean enableShortCircuiting() {
return enableShortCircuiting;
}

Optional<PartialVars> partialVars() {
return Optional.ofNullable(partialVars);
}
Expand All @@ -119,11 +115,12 @@ Optional<PartialVars> partialVars() {

private ExecutionFrame(
CelFunctionResolver functionResolver,
int limit,
CelOptions celOptions,
@Nullable PartialVars partialVars,
@Nullable CelEvaluationListener listener,
@Nullable AsyncCallStateTracker asyncTracker) {
this.comprehensionIterationLimit = limit;
this.comprehensionIterationLimit = getComprehensionMaxIterations(celOptions);
this.enableShortCircuiting = celOptions.enableShortCircuiting();
this.functionResolver = functionResolver;
this.partialVars = partialVars;
this.listener = listener;
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ static Object applyQualifiers(
}
}

return celValueConverter.maybeUnwrap(celValueConverter.toRuntimeValue(obj));
return EvalHelpers.convertAndAdaptResult(celValueConverter, obj);
}

private static Optional<CelAttributePattern> findPartialMatchingPattern(
Expand Down
104 changes: 103 additions & 1 deletion runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ public final class ProgramPlannerTest {
newMemberOverload(
"bytes_concat_bytes", SimpleType.BYTES, SimpleType.BYTES, SimpleType.BYTES)))
.addMessageTypes(TestAllTypes.getDescriptor())
.addLibraries(CelExtensions.optional(), CelExtensions.comprehensions())
.addLibraries(
CelExtensions.optional(), CelExtensions.comprehensions(), CelExtensions.bindings())
.setContainer(CEL_CONTAINER)
.build();

Expand Down Expand Up @@ -200,6 +201,14 @@ private static DefaultDispatcher newDispatcher() {
DescriptorTypeResolver.create(TYPE_PROVIDER, CelValueConverter.getDefaultInstance()));
addBindingsToDispatcher(
builder, typeFunction.newFunctionBindings(CEL_OPTIONS, RUNTIME_EQUALITY));
addBindingsToDispatcher(
builder,
CelFunctionBinding.fromOverloads(
"cel.@mapInsert",
CelFunctionBinding.from(
"cel_@mapInsert_map_key_value",
ImmutableList.of(Map.class, Object.class, Object.class),
args -> args[0])));

// Custom functions
addBindingsToDispatcher(
Expand Down Expand Up @@ -1002,6 +1011,9 @@ public void plan_select_badPresenceTest_throws() throws Exception {
@TestParameters("{expression: '[1,2,3].exists(i, v, i >= 0 && v > 0) == true'}")
@TestParameters("{expression: '[1,2,3].exists(i, v, i < 0 || v < 0) == false'}")
@TestParameters("{expression: '[1,2,3].map(x, x + 1) == [2,3,4]'}")
@TestParameters(
"{expression: 'cel.bind(x, [1, 2], [x + [3], x + [4]]) == [[1, 2, 3], [1, 2, 4]]'}")
@TestParameters("{expression: 'cel.bind(m, {\"a\": 1}, [m]) == [{\"a\": 1}]'}")
public void plan_comprehension_lists(String expression) throws Exception {
CelAbstractSyntaxTree ast = compile(expression);
Program program = PLANNER.plan(ast);
Expand All @@ -1016,6 +1028,7 @@ public void plan_comprehension_lists(String expression) throws Exception {
@TestParameters("{expression: '{\"a\": 1, \"b\": 2}.exists(k, k == \"c\") == false'}")
@TestParameters("{expression: '{\"a\": \"b\", \"c\": \"c\"}.exists(k, v, k == v)'}")
@TestParameters("{expression: '{\"a\": 1, \"b\": 2}.exists(k, v, v == 3) == false'}")
@TestParameters("{expression: '({}.map(k, k) + [1])[0] == 1'}")
public void plan_comprehension_maps(String expression) throws Exception {
CelAbstractSyntaxTree ast = compile(expression);
Program program = PLANNER.plan(ast);
Expand Down Expand Up @@ -2054,6 +2067,95 @@ public void plan_exhaustiveConditional_untakenBranchError_evaluatesSuccessfully(
assertThat(result).isEqualTo(42L);
}

@Test
public void plan_exhaustiveComprehension_filterDoesNotMutateUntakenBranch() throws Exception {
CelAbstractSyntaxTree ast = CEL_COMPILER.compile("[1, 2, 3].filter(x, x > 1)").getAst();
ProgramPlanner planner =
newPlannerWithOptions(CelOptions.current().enableShortCircuiting(false).build());
Program program = planner.plan(ast);

Object result = program.eval();

assertThat(result).isEqualTo(ImmutableList.of(2L, 3L));
}

@Test
@TestParameters("{expression: '[1, 2, 3].map(x, x + 1) == [2, 3, 4]'}")
@TestParameters("{expression: '{\"a\": 1, \"b\": 2}.transformMap(k, v, v) == {}'}")
@SuppressWarnings("Immutable") // Test only
public void plan_comprehension_accumulationSkipsIntermediateConversion(String expression)
throws Exception {
int[] containerConversions = new int[1];
CelValueConverter countingConverter =
new CelValueConverter() {
@Override
public Object toRuntimeValue(Object value) {
if (value instanceof Iterable || value instanceof ImmutableMap) {
containerConversions[0]++;
}
return super.toRuntimeValue(value);
}
};
ProgramPlanner planner =
ProgramPlanner.newPlanner(
TYPE_PROVIDER,
VALUE_PROVIDER,
newDispatcher(),
countingConverter,
CEL_CONTAINER,
CEL_OPTIONS,
ImmutableSet.of(),
RUNTIME_EQUALITY,
CelAsyncEvaluationOptions.defaultOptions(),
/* asyncExecutor= */ null);
Program program = planner.plan(CEL_COMPILER.compile(expression).getAst());

boolean result = (boolean) program.eval();

assertThat(result).isTrue();
assertThat(containerConversions[0]).isEqualTo(1);
}

@Test
public void plan_nonStrictFunction_withErrorArg_adaptsToException() throws Exception {
CelCompiler compiler =
CelCompilerFactory.standardCelCompilerBuilder()
.addFunctionDeclarations(
newFunctionDeclaration(
"is_error", newGlobalOverload("is_error_int", SimpleType.BOOL, SimpleType.INT)))
.build();
DefaultDispatcher.Builder builder = DefaultDispatcher.newBuilder();
addBindingsToDispatcher(
builder,
CelStandardFunctions.newBuilder()
.includeFunctions(StandardFunction.DIVIDE)
.build()
.newFunctionBindings(RUNTIME_EQUALITY, CEL_OPTIONS));
builder.addOverload(
"is_error",
"is_error_int",
ImmutableList.of(Long.class),
/* isStrict= */ false,
args -> args[0] instanceof Exception);
ProgramPlanner planner =
ProgramPlanner.newPlanner(
TYPE_PROVIDER,
VALUE_PROVIDER,
builder.build(),
CEL_VALUE_CONVERTER,
CEL_CONTAINER,
CEL_OPTIONS,
ImmutableSet.of(),
RUNTIME_EQUALITY,
CelAsyncEvaluationOptions.defaultOptions(),
/* asyncExecutor= */ null);
Program program = planner.plan(compiler.compile("is_error(1 / 0)").getAst());

Object result = program.eval();

assertThat(result).isEqualTo(true);
}

private static ProgramPlanner newPlannerWithOptions(CelOptions options) {
return ProgramPlanner.newPlanner(
TYPE_PROVIDER,
Expand Down
Loading