Skip to content

Commit 2d7694a

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
No public description
PiperOrigin-RevId: 1003358263
1 parent 6ef4a7b commit 2d7694a

3 files changed

Lines changed: 346 additions & 26 deletions

File tree

‎optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,13 @@ java_library(
6060
":default_optimizer_constants",
6161
"//:auto_value",
6262
"//bundle:cel",
63+
"//checker:standard_decl",
6364
"//common:cel_ast",
6465
"//common:cel_source",
6566
"//common:compiler_common",
6667
"//common:mutable_ast",
6768
"//common:mutable_source",
69+
"//common:operator",
6870
"//common/ast",
6971
"//common/ast:cel_block",
7072
"//common/ast:mutable_expr",

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

Lines changed: 171 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,18 @@
2929
import com.google.common.base.Verify;
3030
import com.google.common.collect.ImmutableList;
3131
import com.google.common.collect.ImmutableListMultimap;
32+
import com.google.common.collect.ImmutableMap;
3233
import com.google.common.collect.ImmutableSet;
3334
import com.google.common.collect.Multimaps;
3435
import com.google.common.collect.Sets;
3536
import com.google.common.collect.Streams;
3637
import com.google.errorprone.annotations.CanIgnoreReturnValue;
3738
import dev.cel.bundle.Cel;
3839
import dev.cel.bundle.CelBuilder;
40+
import dev.cel.checker.CelStandardDeclarations.StandardFunction.Overload.Arithmetic;
41+
import dev.cel.checker.CelStandardDeclarations.StandardFunction.Overload.BooleanOperator;
42+
import dev.cel.checker.CelStandardDeclarations.StandardFunction.Overload.Comparison;
43+
import dev.cel.checker.CelStandardDeclarations.StandardFunction.Overload.Relation;
3944
import dev.cel.common.CelAbstractSyntaxTree;
4045
import dev.cel.common.CelMutableAst;
4146
import dev.cel.common.CelMutableSource;
@@ -45,15 +50,23 @@
4550
import dev.cel.common.CelSource.Extension.Version;
4651
import dev.cel.common.CelValidationException;
4752
import dev.cel.common.CelVarDecl;
53+
import dev.cel.common.Operator;
4854
import dev.cel.common.ast.CelBlock;
4955
import dev.cel.common.ast.CelExpr;
5056
import dev.cel.common.ast.CelExpr.CelCall;
5157
import dev.cel.common.ast.CelExpr.CelComprehension;
5258
import dev.cel.common.ast.CelExpr.CelList;
5359
import dev.cel.common.ast.CelExpr.ExprKind.Kind;
5460
import dev.cel.common.ast.CelMutableExpr;
61+
import dev.cel.common.ast.CelMutableExpr.CelMutableCall;
62+
import dev.cel.common.ast.CelMutableExpr.CelMutableComprehension;
5563
import dev.cel.common.ast.CelMutableExpr.CelMutableIdent;
64+
import dev.cel.common.ast.CelMutableExpr.CelMutableList;
65+
import dev.cel.common.ast.CelMutableExpr.CelMutableMap;
66+
import dev.cel.common.ast.CelMutableExpr.CelMutableSelect;
67+
import dev.cel.common.ast.CelMutableExpr.CelMutableStruct;
5668
import dev.cel.common.ast.CelMutableExprConverter;
69+
import dev.cel.common.ast.CelReference;
5770
import dev.cel.common.navigation.CelNavigableExpr;
5871
import dev.cel.common.navigation.CelNavigableExprUtil;
5972
import dev.cel.common.navigation.CelNavigableMutableAst;
@@ -66,9 +79,12 @@
6679
import dev.cel.optimizer.CelAstOptimizer;
6780
import java.util.ArrayList;
6881
import java.util.Arrays;
82+
import java.util.Collections;
6983
import java.util.Comparator;
84+
import java.util.EnumSet;
7085
import java.util.HashMap;
7186
import java.util.HashSet;
87+
import java.util.IdentityHashMap;
7288
import java.util.List;
7389
import java.util.Map;
7490
import java.util.Set;
@@ -175,7 +191,9 @@ private OptimizationResult optimizeUsingCelBlock(CelAbstractSyntaxTree ast, Cel
175191

176192
for (iterCount = 0; iterCount < cseOptions.iterationLimit(); iterCount++) {
177193
CelNavigableMutableAst navAst = CelNavigableMutableAst.fromAst(astToModify);
178-
ImmutableList<CelMutableExpr> cseCandidates = getCseCandidates(navAst);
194+
ValueNumberer valueNumberer =
195+
new ValueNumberer(navAst.getRoot().expr(), ast.getReferenceMap());
196+
ImmutableList<CelMutableExpr> cseCandidates = getCseCandidates(navAst, valueNumberer);
179197
if (cseCandidates.isEmpty()) {
180198
break;
181199
}
@@ -392,12 +410,13 @@ private static void replaceWithBlockIdent(
392410
expr.setIdent(CelMutableIdent.create(blockIdentifier));
393411
}
394412

395-
private ImmutableList<CelMutableExpr> getCseCandidates(CelNavigableMutableAst navAst) {
413+
private ImmutableList<CelMutableExpr> getCseCandidates(
414+
CelNavigableMutableAst navAst, ValueNumberer valueNumberer) {
396415
if (cseOptions.subexpressionMaxRecursionDepth() > 0) {
397416
return getCseCandidatesWithRecursionDepth(
398-
navAst, cseOptions.subexpressionMaxRecursionDepth());
417+
navAst, cseOptions.subexpressionMaxRecursionDepth(), valueNumberer);
399418
} else {
400-
return getCseCandidatesWithCommonSubexpr(navAst);
419+
return getCseCandidatesWithCommonSubexpr(navAst, valueNumberer);
401420
}
402421
}
403422

@@ -406,7 +425,7 @@ private ImmutableList<CelMutableExpr> getCseCandidates(CelNavigableMutableAst na
406425
* subexpr found.
407426
*/
408427
private ImmutableList<CelMutableExpr> getCseCandidatesWithRecursionDepth(
409-
CelNavigableMutableAst navAst, int recursionLimit) {
428+
CelNavigableMutableAst navAst, int recursionLimit, ValueNumberer valueNumberer) {
410429
checkArgument(recursionLimit > 0);
411430
Set<CelMutableExpr> ineligibleExprs = getIneligibleExprsFromComprehensionBranches(navAst);
412431
ImmutableList<CelNavigableMutableExpr> descendants =
@@ -422,7 +441,7 @@ private ImmutableList<CelMutableExpr> getCseCandidatesWithRecursionDepth(
422441
}
423442

424443
ImmutableList<CelMutableExpr> cseCandidates =
425-
getCseCandidatesWithCommonSubexpr(descendants, ineligibleExprs);
444+
getCseCandidatesWithCommonSubexpr(descendants, ineligibleExprs, valueNumberer);
426445
if (!cseCandidates.isEmpty()) {
427446
return cseCandidates;
428447
}
@@ -446,31 +465,32 @@ private ImmutableList<CelMutableExpr> getCseCandidatesWithRecursionDepth(
446465
}
447466

448467
private ImmutableList<CelMutableExpr> getCseCandidatesWithCommonSubexpr(
449-
CelNavigableMutableAst navAst) {
468+
CelNavigableMutableAst navAst, ValueNumberer valueNumberer) {
450469
Set<CelMutableExpr> ineligibleExprs = getIneligibleExprsFromComprehensionBranches(navAst);
451470
ImmutableList<CelNavigableMutableExpr> allNodes =
452471
navAst.getRoot().allNodes(TraversalOrder.PRE_ORDER).collect(toImmutableList());
453472

454-
return getCseCandidatesWithCommonSubexpr(allNodes, ineligibleExprs);
473+
return getCseCandidatesWithCommonSubexpr(allNodes, ineligibleExprs, valueNumberer);
455474
}
456475

457476
private ImmutableList<CelMutableExpr> getCseCandidatesWithCommonSubexpr(
458-
ImmutableList<CelNavigableMutableExpr> allNodes, Set<CelMutableExpr> ineligibleExprs) {
477+
ImmutableList<CelNavigableMutableExpr> allNodes,
478+
Set<CelMutableExpr> ineligibleExprs,
479+
ValueNumberer valueNumberer) {
459480
ImmutableListMultimap<Integer, CelNavigableMutableExpr> nodesByHeight =
460481
Multimaps.index(allNodes, CelNavigableMutableExpr::height);
461-
HashSet<CelMutableExpr> semanticallyEqualNodes = new HashSet<>();
482+
Set<Integer> seenVns = new HashSet<>();
462483
for (CelNavigableMutableExpr node : allNodes) {
463484
ImmutableList<CelNavigableMutableExpr> sameHeightNodes = nodesByHeight.get(node.height());
464485
if (sameHeightNodes.size() < 2 || !canEliminate(node, ineligibleExprs)) {
465486
continue;
466487
}
467-
// Normalize the expr to test semantic equivalence.
468-
CelMutableExpr normalizedExpr = normalizeForEquality(node.expr());
469-
if (!semanticallyEqualNodes.add(normalizedExpr)) {
488+
int vn = valueNumberer.vnOf(node.expr());
489+
if (!seenVns.add(vn)) {
470490
return sameHeightNodes.stream()
471491
.filter(candidate -> canEliminate(candidate, ineligibleExprs))
472492
.map(CelNavigableMutableExpr::expr)
473-
.filter(expr -> normalizeForEquality(expr).equals(normalizedExpr))
493+
.filter(expr -> valueNumberer.vnOf(expr) == vn)
474494
.collect(toImmutableList());
475495
}
476496
}
@@ -550,17 +570,144 @@ private boolean containsEliminableFunctionOnly(CelNavigableMutableExpr navigable
550570
});
551571
}
552572

553-
/**
554-
* Converts the {@link CelMutableExpr} to make it suitable for performing a semantically equals
555-
* check.
556-
*
557-
* <p>Specifically, this will deep copy the mutable expr then set all expr IDs in the expression
558-
* tree to 0.
559-
*/
560-
private CelMutableExpr normalizeForEquality(CelMutableExpr mutableExpr) {
561-
CelMutableExpr copiedExpr = CelMutableExpr.newInstance(mutableExpr);
573+
// Bottom-up hash-consing value numberer. Binary calls f(a, b) and SWAPPED_FUNCTIONS[f](b, a)
574+
// share a value number when the checker resolved them only to standard overloads, as custom
575+
// overloads need not be commutative or converse.
576+
private static final class ValueNumberer {
577+
private static final ImmutableMap<String, String> SWAPPED_FUNCTIONS =
578+
ImmutableMap.<String, String>builder()
579+
.put(Operator.ADD.getFunction(), Operator.ADD.getFunction())
580+
.put(Operator.MULTIPLY.getFunction(), Operator.MULTIPLY.getFunction())
581+
.put(Operator.EQUALS.getFunction(), Operator.EQUALS.getFunction())
582+
.put(Operator.NOT_EQUALS.getFunction(), Operator.NOT_EQUALS.getFunction())
583+
.put(Operator.LOGICAL_AND.getFunction(), Operator.LOGICAL_AND.getFunction())
584+
.put(Operator.LOGICAL_OR.getFunction(), Operator.LOGICAL_OR.getFunction())
585+
.put(Operator.GREATER.getFunction(), Operator.LESS.getFunction())
586+
.put(Operator.GREATER_EQUALS.getFunction(), Operator.LESS_EQUALS.getFunction())
587+
.buildOrThrow();
588+
private static final ImmutableSet<String> SWAPPABLE_OVERLOAD_IDS =
589+
Stream.concat(
590+
Stream.of(
591+
Arithmetic.ADD_INT64,
592+
Arithmetic.ADD_UINT64,
593+
Arithmetic.ADD_DOUBLE,
594+
Arithmetic.ADD_DURATION_DURATION,
595+
Arithmetic.ADD_TIMESTAMP_DURATION,
596+
Arithmetic.ADD_DURATION_TIMESTAMP,
597+
Arithmetic.MULTIPLY_INT64,
598+
Arithmetic.MULTIPLY_UINT64,
599+
Arithmetic.MULTIPLY_DOUBLE,
600+
Relation.EQUALS,
601+
Relation.NOT_EQUALS,
602+
BooleanOperator.LOGICAL_AND,
603+
BooleanOperator.LOGICAL_OR),
604+
EnumSet.allOf(Comparison.class).stream())
605+
.map(overload -> overload.celOverloadDecl().overloadId())
606+
.collect(toImmutableSet());
607+
608+
private final ImmutableMap<Long, CelReference> references;
609+
private final Map<ImmutableList<?>, Integer> keyToVn;
610+
private final IdentityHashMap<CelMutableExpr, Integer> exprToVn;
611+
612+
private int vnOf(CelMutableExpr expr) {
613+
return Verify.verifyNotNull(exprToVn.get(expr));
614+
}
562615

563-
return astMutator.clearExprIds(copiedExpr);
616+
private int computeVn(CelMutableExpr expr) {
617+
ImmutableList<?> key = buildKey(expr);
618+
int vn = keyToVn.computeIfAbsent(key, k -> keyToVn.size());
619+
exprToVn.put(expr, vn);
620+
return vn;
621+
}
622+
623+
private ImmutableList<?> buildKey(CelMutableExpr expr) {
624+
switch (expr.getKind()) {
625+
case CONSTANT:
626+
return ImmutableList.of(Kind.CONSTANT, expr.constant());
627+
case IDENT:
628+
return ImmutableList.of(Kind.IDENT, expr.ident().name());
629+
case SELECT:
630+
CelMutableSelect select = expr.select();
631+
return ImmutableList.of(
632+
Kind.SELECT, computeVn(select.operand()), select.field(), select.testOnly());
633+
case CALL:
634+
CelMutableCall call = expr.call();
635+
String function = call.function();
636+
// VNs are non-negative, so -1 marks a global call.
637+
int targetVn = call.target().isPresent() ? computeVn(call.target().get()) : -1;
638+
List<Integer> argVns = new ArrayList<>(call.args().size());
639+
for (CelMutableExpr arg : call.args()) {
640+
argVns.add(computeVn(arg));
641+
}
642+
String swapped = SWAPPED_FUNCTIONS.get(function);
643+
if (swapped != null
644+
&& argVns.size() == 2
645+
&& hasOnlySwappableOverloads(expr.id())
646+
&& (!swapped.equals(function) || argVns.get(0) > argVns.get(1))) {
647+
function = swapped;
648+
Collections.swap(argVns, 0, 1);
649+
}
650+
return ImmutableList.of(Kind.CALL, targetVn, function, ImmutableList.copyOf(argVns));
651+
case LIST:
652+
CelMutableList list = expr.list();
653+
ImmutableList.Builder<Integer> elemVns =
654+
ImmutableList.builderWithExpectedSize(list.elements().size());
655+
for (CelMutableExpr elem : list.elements()) {
656+
elemVns.add(computeVn(elem));
657+
}
658+
return ImmutableList.of(
659+
Kind.LIST, elemVns.build(), ImmutableList.copyOf(list.optionalIndices()));
660+
case MAP:
661+
CelMutableMap map = expr.map();
662+
ImmutableList.Builder<Object> entryVns =
663+
ImmutableList.builderWithExpectedSize(map.entries().size());
664+
for (CelMutableMap.Entry entry : map.entries()) {
665+
entryVns.add(
666+
ImmutableList.of(
667+
computeVn(entry.key()), computeVn(entry.value()), entry.optionalEntry()));
668+
}
669+
return ImmutableList.of(Kind.MAP, entryVns.build());
670+
case STRUCT:
671+
CelMutableStruct struct = expr.struct();
672+
ImmutableList.Builder<Object> fieldVns =
673+
ImmutableList.builderWithExpectedSize(struct.entries().size());
674+
for (CelMutableStruct.Entry entry : struct.entries()) {
675+
fieldVns.add(
676+
ImmutableList.of(
677+
entry.fieldKey(), computeVn(entry.value()), entry.optionalEntry()));
678+
}
679+
return ImmutableList.of(Kind.STRUCT, struct.messageName(), fieldVns.build());
680+
case COMPREHENSION:
681+
CelMutableComprehension comp = expr.comprehension();
682+
return ImmutableList.of(
683+
Kind.COMPREHENSION,
684+
comp.iterVar(),
685+
comp.iterVar2(),
686+
comp.accuVar(),
687+
computeVn(comp.iterRange()),
688+
computeVn(comp.accuInit()),
689+
computeVn(comp.loopCondition()),
690+
computeVn(comp.loopStep()),
691+
computeVn(comp.result()));
692+
default:
693+
throw new IllegalArgumentException("Unexpected ExprKind: " + expr.getKind());
694+
}
695+
}
696+
697+
private boolean hasOnlySwappableOverloads(long id) {
698+
CelReference reference = references.get(id);
699+
return reference != null
700+
&& !reference.overloadIds().isEmpty()
701+
&& SWAPPABLE_OVERLOAD_IDS.containsAll(reference.overloadIds());
702+
}
703+
704+
@SuppressWarnings("ValueClassIdentity") // Integer values are only compared by equals()
705+
private ValueNumberer(CelMutableExpr root, ImmutableMap<Long, CelReference> references) {
706+
this.references = references;
707+
this.keyToVn = new HashMap<>();
708+
this.exprToVn = new IdentityHashMap<>();
709+
computeVn(root);
710+
}
564711
}
565712

566713
/** Options to configure how Common Subexpression Elimination behave. */

0 commit comments

Comments
 (0)