Skip to content

Commit 9c562a3

Browse files
jnthntatumcopybara-github
authored andcommitted
Track dirty state in FlatExpressionEvaluatorState to avoid unnecessary resets.
PiperOrigin-RevId: 1003367762
1 parent e51ef9d commit 9c562a3

3 files changed

Lines changed: 12 additions & 3 deletions

File tree

‎eval/compiler/constant_folding.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,18 +201,18 @@ absl::Status ConstantFoldingExtension::OnPostVisit(PlannerContext& context,
201201
// This subexpression is already optimized out or suppressed.
202202
return absl::OkStatus();
203203
}
204-
// copy string to managed handle if backed by the original program.
204+
// Ensure that string constants are copied to the constant folding arena
205205
Value value;
206206
if (node.has_const_expr()) {
207207
CEL_ASSIGN_OR_RETURN(value,
208208
ConvertConstant(node.const_expr(), state_.arena()));
209209
} else {
210-
ExecutionFrame frame(subplan, empty_, context.options(), state_);
211210
state_.Reset();
212211
// Update stack size to accommodate sub expression.
213212
// This only results in a vector resize if the new maxsize is greater than
214213
// the current capacity.
215214
state_.value_stack().SetMaxSize(subplan.size());
215+
ExecutionFrame frame(subplan, empty_, context.options(), state_);
216216

217217
auto result = frame.Evaluate();
218218
// If this would be a runtime error, then don't adjust the program plan, but

‎eval/eval/evaluator_core.cc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,13 @@
4444
namespace google::api::expr::runtime {
4545

4646
void FlatExpressionEvaluatorState::Reset() {
47+
if (!dirty_) {
48+
return;
49+
}
4750
value_stack_.Clear();
4851
iterator_stack_.Clear();
4952
comprehension_slots_.Reset();
53+
dirty_ = false;
5054
}
5155

5256
const ExpressionStep* ExecutionFrame::NextSlow() {

‎eval/eval/evaluator_core.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,7 @@ class FlatExpressionEvaluatorState {
488488
arena_(arena) {}
489489

490490
void Reset();
491+
void MarkDirty() { dirty_ = true; }
491492

492493
EvaluatorStack& value_stack() { return value_stack_; }
493494

@@ -527,6 +528,7 @@ class FlatExpressionEvaluatorState {
527528
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool_;
528529
google::protobuf::MessageFactory* absl_nullability_unknown message_factory_ = nullptr;
529530
google::protobuf::Arena* absl_nullability_unknown arena_ = nullptr;
531+
bool dirty_ = false;
530532
};
531533

532534
// Context needed for evaluation. This is sufficient for supporting
@@ -728,7 +730,9 @@ class ExecutionFrame : public ExecutionFrameBase {
728730
execution_path_(flat),
729731
value_stack_(&state.value_stack()),
730732
iterator_stack_(&state.iterator_stack()),
731-
subexpressions_(&execution_path_, 1) {}
733+
subexpressions_(&execution_path_, 1) {
734+
state.MarkDirty();
735+
}
732736

733737
ExecutionFrame(
734738
absl::Span<const ExecutionPathView> subexpressions,
@@ -746,6 +750,7 @@ class ExecutionFrame : public ExecutionFrameBase {
746750
iterator_stack_(&state.iterator_stack()),
747751
subexpressions_(subexpressions) {
748752
ABSL_DCHECK(!subexpressions.empty());
753+
state.MarkDirty();
749754
}
750755

751756
// Returns next expression to evaluate.

0 commit comments

Comments
 (0)