From 18b0dddad11fbc1527655688b604a9a29462d753 Mon Sep 17 00:00:00 2001 From: Blaz Snuderl Date: Sun, 26 Apr 2026 13:22:18 +0200 Subject: [PATCH] Skip the now binding when no program references it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Variable.newThisVariable always allocated three objects per field evaluation (a NowVariable, the this Variable, and a hierarchical wrapper). Almost no rule expression references the now identifier, so the wrapping is pure waste in the common case. Walk each compiled AST once at build time to record whether it references now. CelPrograms aggregates that flag across its program list and, when nothing needs now, asks Variable for a single unwrapped resolver — three allocations become one per field eval. --- .../build/buf/protovalidate/AstExpression.java | 12 ++++++++++++ .../build/buf/protovalidate/CelPrograms.java | 17 ++++++++++++++++- .../buf/protovalidate/CompiledProgram.java | 12 +++++++++++- .../buf/protovalidate/EvaluatorBuilder.java | 5 ++++- .../java/build/buf/protovalidate/RuleCache.java | 15 ++++++++++++--- .../java/build/buf/protovalidate/Variable.java | 17 +++++++++++------ 6 files changed, 66 insertions(+), 12 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/AstExpression.java b/src/main/java/build/buf/protovalidate/AstExpression.java index 829925b5..7cf46cc0 100644 --- a/src/main/java/build/buf/protovalidate/AstExpression.java +++ b/src/main/java/build/buf/protovalidate/AstExpression.java @@ -18,6 +18,8 @@ import dev.cel.common.CelAbstractSyntaxTree; import dev.cel.common.CelValidationException; import dev.cel.common.CelValidationResult; +import dev.cel.common.ast.CelExpr.ExprKind; +import dev.cel.common.navigation.CelNavigableAst; import dev.cel.common.types.CelKind; import dev.cel.compiler.CelCompiler; @@ -67,4 +69,14 @@ static AstExpression newAstExpression(CelCompiler cel, Expression expr) } return new AstExpression(ast, expr); } + + /** Returns true if the AST references the given identifier name anywhere in its tree. */ + static boolean referencesIdentifier(CelAbstractSyntaxTree ast, String name) { + return CelNavigableAst.fromAst(ast) + .getRoot() + .allNodes() + .anyMatch( + node -> + node.getKind() == ExprKind.Kind.IDENT && node.expr().ident().name().equals(name)); + } } diff --git a/src/main/java/build/buf/protovalidate/CelPrograms.java b/src/main/java/build/buf/protovalidate/CelPrograms.java index ed712f79..ac5a18fe 100644 --- a/src/main/java/build/buf/protovalidate/CelPrograms.java +++ b/src/main/java/build/buf/protovalidate/CelPrograms.java @@ -27,6 +27,13 @@ final class CelPrograms implements Evaluator { /** A list of {@link CompiledProgram} that will be executed against the input message. */ private final List programs; + /** + * Whether any program in {@link #programs} references the {@code now} variable. Computed once at + * construction so {@link #evaluate} can skip the {@link NowVariable} wrapper when no program + * needs it. + */ + private final boolean anyUsesNow; + /** * Constructs a new {@link CelPrograms}. * @@ -35,6 +42,14 @@ final class CelPrograms implements Evaluator { CelPrograms(@Nullable ValueEvaluator valueEvaluator, List compiledPrograms) { this.helper = new RuleViolationHelper(valueEvaluator); this.programs = compiledPrograms; + boolean anyUsesNow = false; + for (CompiledProgram program : compiledPrograms) { + if (program.usesNow()) { + anyUsesNow = true; + break; + } + } + this.anyUsesNow = anyUsesNow; } @Override @@ -45,7 +60,7 @@ public boolean tautology() { @Override public List evaluate(Value val, boolean failFast) throws ExecutionException { - CelVariableResolver bindings = Variable.newThisVariable(val.value(Object.class)); + CelVariableResolver bindings = Variable.newThisVariable(val.value(Object.class), anyUsesNow); List violations = new ArrayList<>(); for (CompiledProgram program : programs) { RuleViolation.Builder violation = program.eval(val, bindings); diff --git a/src/main/java/build/buf/protovalidate/CompiledProgram.java b/src/main/java/build/buf/protovalidate/CompiledProgram.java index 779b0bd7..c8d369c1 100644 --- a/src/main/java/build/buf/protovalidate/CompiledProgram.java +++ b/src/main/java/build/buf/protovalidate/CompiledProgram.java @@ -44,6 +44,9 @@ final class CompiledProgram { */ @Nullable private final CelVariableResolver globals; + /** Whether the compiled expression references the {@code now} variable. */ + private final boolean usesNow; + /** * Constructs a new {@link CompiledProgram}. * @@ -51,18 +54,25 @@ final class CompiledProgram { * @param source The original expression that was compiled into the program. * @param rulePath The field path from the FieldRules to the rule value. * @param ruleValue The rule value. + * @param usesNow Whether the source expression references the {@code now} variable. */ CompiledProgram( Program program, Expression source, @Nullable FieldPath rulePath, @Nullable Value ruleValue, - @Nullable CelVariableResolver globals) { + @Nullable CelVariableResolver globals, + boolean usesNow) { this.program = program; this.source = source; this.rulePath = rulePath; this.ruleValue = ruleValue; this.globals = globals; + this.usesNow = usesNow; + } + + boolean usesNow() { + return usesNow; } /** diff --git a/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java b/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java index 05f8c1df..85a945ae 100644 --- a/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java +++ b/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java @@ -544,13 +544,16 @@ private static List compileRules( FieldPath.newBuilder().addElements(fieldPathElement.toBuilder().setIndex(i)).build(); } try { + boolean usesNow = + AstExpression.referencesIdentifier(astExpression.ast, NowVariable.NOW_NAME); compiledPrograms.add( new CompiledProgram( cel.createProgram(astExpression.ast), astExpression.source, rulePath, new MessageValue(rules.get(i)), - null)); + null, + usesNow)); } catch (CelEvaluationException e) { throw new CompilationException("failed to evaluate rule " + rules.get(i).getId(), e); } diff --git a/src/main/java/build/buf/protovalidate/RuleCache.java b/src/main/java/build/buf/protovalidate/RuleCache.java index 9e6962bf..7559f11f 100644 --- a/src/main/java/build/buf/protovalidate/RuleCache.java +++ b/src/main/java/build/buf/protovalidate/RuleCache.java @@ -45,13 +45,19 @@ private static class CelRule { final Program program; final FieldDescriptor field; final FieldPath rulePath; + final boolean usesNow; private CelRule( - AstExpression astExpression, Program program, FieldDescriptor field, FieldPath rulePath) { + AstExpression astExpression, + Program program, + FieldDescriptor field, + FieldPath rulePath, + boolean usesNow) { this.astExpression = astExpression; this.program = program; this.field = field; this.rulePath = rulePath; + this.usesNow = usesNow; } } @@ -129,7 +135,8 @@ List compile( rule.astExpression.source, rule.rulePath, new ObjectValue(rule.field, fieldValue), - Variable.newRuleVariable(message, ProtoAdapter.toCel(rule.field, fieldValue)))); + Variable.newRuleVariable(message, ProtoAdapter.toCel(rule.field, fieldValue)), + rule.usesNow)); } return Collections.unmodifiableList(programs); } @@ -187,7 +194,9 @@ private List buildCelRules( throw new CompilationException( "failed to create program for rule " + astExpression.source.id, e); } - celRules.add(new CelRule(astExpression, program, ruleFieldDesc, rulePath)); + boolean usesNow = + AstExpression.referencesIdentifier(astExpression.ast, NowVariable.NOW_NAME); + celRules.add(new CelRule(astExpression, program, ruleFieldDesc, rulePath, usesNow)); } return celRules; } diff --git a/src/main/java/build/buf/protovalidate/Variable.java b/src/main/java/build/buf/protovalidate/Variable.java index 592d2032..75248ccb 100644 --- a/src/main/java/build/buf/protovalidate/Variable.java +++ b/src/main/java/build/buf/protovalidate/Variable.java @@ -45,14 +45,19 @@ private Variable(String name, @Nullable Object val) { } /** - * Creates a "this" variable. + * Creates a resolver for the {@code this} variable. When {@code includeNow} is false the {@code + * now} resolver and the hierarchical wrapper are skipped — three allocations become one. * - * @param val the value. - * @return {@link Variable}. + * @param val the value bound to {@code this}. + * @param includeNow whether the resolver should also expose the {@code now} variable. + * @return a {@link CelVariableResolver} bound to {@code this} (and {@code now} when requested). */ - static CelVariableResolver newThisVariable(@Nullable Object val) { - return CelVariableResolver.hierarchicalVariableResolver( - new NowVariable(), new Variable(THIS_NAME, val)); + static CelVariableResolver newThisVariable(@Nullable Object val, boolean includeNow) { + Variable thisVar = new Variable(THIS_NAME, val); + if (!includeNow) { + return thisVar; + } + return CelVariableResolver.hierarchicalVariableResolver(new NowVariable(), thisVar); } /**