Skip to content

Commit bc4256d

Browse files
l46kokcopybara-github
authored andcommitted
Remove subexpressionMaxRecursionDepth from SubexpressionOptimizer.
PiperOrigin-RevId: 1004362319
1 parent 356b616 commit bc4256d

18 files changed

Lines changed: 146 additions & 56131 deletions

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

Lines changed: 1 addition & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
package dev.cel.optimizer.optimizers;
1616

17-
import static com.google.common.base.Preconditions.checkArgument;
1817
import static com.google.common.base.Preconditions.checkNotNull;
1918
import static com.google.common.base.Preconditions.checkState;
2019
import static com.google.common.collect.ImmutableList.toImmutableList;
@@ -66,7 +65,6 @@
6665
import dev.cel.optimizer.CelAstOptimizer;
6766
import java.util.ArrayList;
6867
import java.util.Arrays;
69-
import java.util.Comparator;
7068
import java.util.HashMap;
7169
import java.util.HashSet;
7270
import java.util.List;
@@ -393,69 +391,9 @@ private static void replaceWithBlockIdent(
393391
}
394392

395393
private ImmutableList<CelMutableExpr> getCseCandidates(CelNavigableMutableAst navAst) {
396-
if (cseOptions.subexpressionMaxRecursionDepth() > 0) {
397-
return getCseCandidatesWithRecursionDepth(
398-
navAst, cseOptions.subexpressionMaxRecursionDepth());
399-
} else {
400-
return getCseCandidatesWithCommonSubexpr(navAst);
401-
}
402-
}
403-
404-
/**
405-
* Retrieves all subexpr candidates based on the recursion limit even if there's no duplicate
406-
* subexpr found.
407-
*/
408-
private ImmutableList<CelMutableExpr> getCseCandidatesWithRecursionDepth(
409-
CelNavigableMutableAst navAst, int recursionLimit) {
410-
checkArgument(recursionLimit > 0);
411-
Set<CelMutableExpr> ineligibleExprs = getIneligibleExprsFromComprehensionBranches(navAst);
412-
ImmutableList<CelNavigableMutableExpr> descendants =
413-
navAst
414-
.getRoot()
415-
.descendants(TraversalOrder.PRE_ORDER)
416-
.filter(node -> node.height() <= recursionLimit)
417-
.filter(node -> canEliminate(node, ineligibleExprs))
418-
.sorted(Comparator.comparingInt(CelNavigableMutableExpr::height).reversed())
419-
.collect(toImmutableList());
420-
if (descendants.isEmpty()) {
421-
return ImmutableList.of();
422-
}
423-
424-
ImmutableList<CelMutableExpr> cseCandidates =
425-
getCseCandidatesWithCommonSubexpr(descendants, ineligibleExprs);
426-
if (!cseCandidates.isEmpty()) {
427-
return cseCandidates;
428-
}
429-
430-
// If there's no common subexpr, just return the one with the highest height that's still below
431-
// the recursion limit, but only if it actually needs to be extracted due to exceeding the
432-
// recursion limit.
433-
boolean astHasMoreExtractableSubexprs =
434-
navAst
435-
.getRoot()
436-
.allNodes(TraversalOrder.POST_ORDER)
437-
.filter(node -> node.height() > recursionLimit)
438-
.anyMatch(node -> canEliminate(node, ineligibleExprs));
439-
if (astHasMoreExtractableSubexprs) {
440-
return ImmutableList.of(descendants.get(0).expr());
441-
}
442-
443-
// The height of the remaining subexpression is already below the recursion limit. No need to
444-
// extract.
445-
return ImmutableList.of();
446-
}
447-
448-
private ImmutableList<CelMutableExpr> getCseCandidatesWithCommonSubexpr(
449-
CelNavigableMutableAst navAst) {
450394
Set<CelMutableExpr> ineligibleExprs = getIneligibleExprsFromComprehensionBranches(navAst);
451395
ImmutableList<CelNavigableMutableExpr> allNodes =
452396
navAst.getRoot().allNodes(TraversalOrder.PRE_ORDER).collect(toImmutableList());
453-
454-
return getCseCandidatesWithCommonSubexpr(allNodes, ineligibleExprs);
455-
}
456-
457-
private ImmutableList<CelMutableExpr> getCseCandidatesWithCommonSubexpr(
458-
ImmutableList<CelNavigableMutableExpr> allNodes, Set<CelMutableExpr> ineligibleExprs) {
459397
ImmutableListMultimap<Integer, CelNavigableMutableExpr> nodesByHeight =
460398
Multimaps.index(allNodes, CelNavigableMutableExpr::height);
461399
HashSet<CelMutableExpr> semanticallyEqualNodes = new HashSet<>();
@@ -572,8 +510,6 @@ public abstract static class SubexpressionOptimizerOptions {
572510

573511
public abstract boolean enableCelBlock();
574512

575-
public abstract int subexpressionMaxRecursionDepth();
576-
577513
public abstract ImmutableSet<String> eliminableFunctions();
578514

579515
/** Builder for configuring the {@link SubexpressionOptimizerOptions}. */
@@ -598,33 +534,6 @@ public abstract static class Builder {
598534
@Deprecated
599535
public abstract Builder enableCelBlock(boolean value);
600536

601-
/**
602-
* Ensures all extracted subexpressions do not exceed the maximum depth of designated value.
603-
* The purpose of this is to guarantee evaluation and deserialization safety by preventing
604-
* deeply nested ASTs. The trade-off is increased memory usage due to memoizing additional
605-
* block indices during lazy evaluation.
606-
*
607-
* <p>As a general note, root of a node has a depth of 0. An expression `x.y.z` has a depth of
608-
* 2.
609-
*
610-
* <p>Note that expressions containing no common subexpressions may become a candidate for
611-
* extraction to satisfy the max depth requirement.
612-
*
613-
* <p>This is a no-op if the configured value is less than 1, or no subexpression needs to be
614-
* extracted because the entire expression is already under the designated limit.
615-
*
616-
* <p>Examples:
617-
*
618-
* <ol>
619-
* <li>a.b.c with depth 1 -> cel.@block([x.b, @index0.c], @index1)
620-
* <li>a.b.c with depth 3 -> a.b.c
621-
* <li>a.b + a.b.c.d with depth 3 -> cel.@block([a.b, @index0.c.d], @index0 + @index1)
622-
* </ol>
623-
*
624-
* <p>
625-
*/
626-
public abstract Builder subexpressionMaxRecursionDepth(int value);
627-
628537
abstract ImmutableSet.Builder<String> eliminableFunctionsBuilder();
629538

630539
/**
@@ -651,15 +560,12 @@ public Builder addEliminableFunctions(String... functions) {
651560
Builder() {}
652561
}
653562

654-
abstract Builder toBuilder();
655-
656563
/** Returns a new options builder with recommended defaults pre-configured. */
657564
public static Builder newBuilder() {
658565
return new AutoValue_SubexpressionOptimizer_SubexpressionOptimizerOptions.Builder()
659566
.iterationLimit(500)
660567
.enableCelBlock(true)
661-
.populateMacroCalls(false)
662-
.subexpressionMaxRecursionDepth(0);
568+
.populateMacroCalls(false);
663569
}
664570

665571
SubexpressionOptimizerOptions() {}

0 commit comments

Comments
 (0)