diff --git a/optimizer/src/main/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizer.java b/optimizer/src/main/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizer.java index 181cc4f75..aa724fdb6 100644 --- a/optimizer/src/main/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizer.java +++ b/optimizer/src/main/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizer.java @@ -121,6 +121,9 @@ private static CelMutableExpr newOptionalNoneExpr() { } @Override + // Internal optimization steps preserve the initial mutable AST instance if no mutations occur. + // Using == avoids deep .equals() comparison. + @SuppressWarnings("ReferenceEquality") public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel) throws CelOptimizationException { CelBuilder builder = cel.toCelBuilder(); @@ -134,12 +137,17 @@ public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel) // Override the environment's expected type to generally allow all subtrees to be folded. Cel optimizerEnv = builder.setResultType(SimpleType.DYN).build(); - CelMutableAst mutableAst = CelMutableAst.fromCelAst(ast); - ImmutableMap identTypes = precomputeIdentTypes(mutableAst); + CelMutableAst initialMutableAst = CelMutableAst.fromCelAst(ast); + ImmutableMap identTypes = precomputeIdentTypes(initialMutableAst); - mutableAst = foldConstants(optimizerEnv, valueProvider, identTypes, mutableAst); + CelMutableAst mutableAst = + foldConstants(optimizerEnv, valueProvider, identTypes, initialMutableAst); mutableAst = pruneOptionalElements(mutableAst); + if (mutableAst == initialMutableAst) { + return OptimizationResult.create(ast); + } + return OptimizationResult.create(astMutator.renumberIdsConsecutively(mutableAst).toParsedAst()); } @@ -735,10 +743,21 @@ private CelMutableAst pruneOptionalListElements(CelMutableAst mutableAst, CelMut updatedIndicesBuilder.add(newOptIndex); } + // An optional list is modified if: + // 1. An optional.none() was dropped - it this case, the updatedElements.size() decreases. + // 2. An optional.of(literal) was unwrapped into a regular element - in this case, + // updatedIndices.size() decreases. + // If both counts are unchanged, neither case occurred, and we can return the original AST. + ImmutableList updatedElements = updatedElemBuilder.build(); + ImmutableList updatedIndices = updatedIndicesBuilder.build(); + if (updatedElements.size() == list.elements().size() + && updatedIndices.size() == list.optionalIndices().size()) { + return mutableAst; + } + return astMutator.replaceSubtree( mutableAst, - CelMutableExpr.ofList( - CelMutableList.create(updatedElemBuilder.build(), updatedIndicesBuilder.build())), + CelMutableExpr.ofList(CelMutableList.create(updatedElements, updatedIndices)), expr.id()); } diff --git a/optimizer/src/main/java/dev/cel/optimizer/optimizers/InliningOptimizer.java b/optimizer/src/main/java/dev/cel/optimizer/optimizers/InliningOptimizer.java index 61fd19347..696b6749b 100644 --- a/optimizer/src/main/java/dev/cel/optimizer/optimizers/InliningOptimizer.java +++ b/optimizer/src/main/java/dev/cel/optimizer/optimizers/InliningOptimizer.java @@ -101,8 +101,12 @@ public static InliningOptimizer newInstance( } @Override + // Internal optimization steps preserve the initial mutable AST instance if no mutations occur. + // Using == avoids deep .equals() comparison. + @SuppressWarnings("ReferenceEquality") public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel) { - CelMutableAst mutableAst = CelMutableAst.fromCelAst(ast); + CelMutableAst initialMutableAst = CelMutableAst.fromCelAst(ast); + CelMutableAst mutableAst = initialMutableAst; for (InlineVariable inlineVariable : inlineVariables) { mutableAst = astMutator.mutateUntilFixedPoint( @@ -125,6 +129,10 @@ public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel) { }); } + if (mutableAst == initialMutableAst) { + return OptimizationResult.create(ast); + } + return OptimizationResult.create(astMutator.renumberIdsConsecutively(mutableAst).toParsedAst()); }