Skip to content

Commit b0cff2c

Browse files
Cristina Borzacopybara-github
authored andcommitted
Avoid unnecessary AST renumbering in CEL optimizers when no changes are made.
PiperOrigin-RevId: 976189920
1 parent fbe7f97 commit b0cff2c

2 files changed

Lines changed: 33 additions & 6 deletions

File tree

‎optimizer/src/main/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizer.java‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ private static CelMutableExpr newOptionalNoneExpr() {
121121
}
122122

123123
@Override
124+
// Internal optimization steps preserve the initial mutable AST instance if no mutations occur.
125+
// Using == avoids deep .equals() comparison.
126+
@SuppressWarnings("ReferenceEquality")
124127
public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel)
125128
throws CelOptimizationException {
126129
CelBuilder builder = cel.toCelBuilder();
@@ -134,12 +137,17 @@ public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel)
134137
// Override the environment's expected type to generally allow all subtrees to be folded.
135138
Cel optimizerEnv = builder.setResultType(SimpleType.DYN).build();
136139

137-
CelMutableAst mutableAst = CelMutableAst.fromCelAst(ast);
138-
ImmutableMap<String, CelType> identTypes = precomputeIdentTypes(mutableAst);
140+
CelMutableAst initialMutableAst = CelMutableAst.fromCelAst(ast);
141+
ImmutableMap<String, CelType> identTypes = precomputeIdentTypes(initialMutableAst);
139142

140-
mutableAst = foldConstants(optimizerEnv, valueProvider, identTypes, mutableAst);
143+
CelMutableAst mutableAst =
144+
foldConstants(optimizerEnv, valueProvider, identTypes, initialMutableAst);
141145
mutableAst = pruneOptionalElements(mutableAst);
142146

147+
if (mutableAst == initialMutableAst) {
148+
return OptimizationResult.create(ast);
149+
}
150+
143151
return OptimizationResult.create(astMutator.renumberIdsConsecutively(mutableAst).toParsedAst());
144152
}
145153

@@ -735,10 +743,21 @@ private CelMutableAst pruneOptionalListElements(CelMutableAst mutableAst, CelMut
735743
updatedIndicesBuilder.add(newOptIndex);
736744
}
737745

746+
// An optional list is modified if:
747+
// 1. An optional.none() was dropped - it this case, the updatedElements.size() decreases.
748+
// 2. An optional.of(literal) was unwrapped into a regular element - in this case,
749+
// updatedIndices.size() decreases.
750+
// If both counts are unchanged, neither case occurred, and we can return the original AST.
751+
ImmutableList<CelMutableExpr> updatedElements = updatedElemBuilder.build();
752+
ImmutableList<Integer> updatedIndices = updatedIndicesBuilder.build();
753+
if (updatedElements.size() == list.elements().size()
754+
&& updatedIndices.size() == list.optionalIndices().size()) {
755+
return mutableAst;
756+
}
757+
738758
return astMutator.replaceSubtree(
739759
mutableAst,
740-
CelMutableExpr.ofList(
741-
CelMutableList.create(updatedElemBuilder.build(), updatedIndicesBuilder.build())),
760+
CelMutableExpr.ofList(CelMutableList.create(updatedElements, updatedIndices)),
742761
expr.id());
743762
}
744763

‎optimizer/src/main/java/dev/cel/optimizer/optimizers/InliningOptimizer.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,12 @@ public static InliningOptimizer newInstance(
101101
}
102102

103103
@Override
104+
// Internal optimization steps preserve the initial mutable AST instance if no mutations occur.
105+
// Using == avoids deep .equals() comparison.
106+
@SuppressWarnings("ReferenceEquality")
104107
public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel) {
105-
CelMutableAst mutableAst = CelMutableAst.fromCelAst(ast);
108+
CelMutableAst initialMutableAst = CelMutableAst.fromCelAst(ast);
109+
CelMutableAst mutableAst = initialMutableAst;
106110
for (InlineVariable inlineVariable : inlineVariables) {
107111
mutableAst =
108112
astMutator.mutateUntilFixedPoint(
@@ -125,6 +129,10 @@ public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel) {
125129
});
126130
}
127131

132+
if (mutableAst == initialMutableAst) {
133+
return OptimizationResult.create(ast);
134+
}
135+
128136
return OptimizationResult.create(astMutator.renumberIdsConsecutively(mutableAst).toParsedAst());
129137
}
130138

0 commit comments

Comments
 (0)