diff --git a/eval/compiler/constant_folding.cc b/eval/compiler/constant_folding.cc index 4a8d55c31..8e854a867 100644 --- a/eval/compiler/constant_folding.cc +++ b/eval/compiler/constant_folding.cc @@ -201,18 +201,18 @@ absl::Status ConstantFoldingExtension::OnPostVisit(PlannerContext& context, // This subexpression is already optimized out or suppressed. return absl::OkStatus(); } - // copy string to managed handle if backed by the original program. + // Ensure that string constants are copied to the constant folding arena Value value; if (node.has_const_expr()) { CEL_ASSIGN_OR_RETURN(value, ConvertConstant(node.const_expr(), state_.arena())); } else { - ExecutionFrame frame(subplan, empty_, context.options(), state_); state_.Reset(); // Update stack size to accommodate sub expression. // This only results in a vector resize if the new maxsize is greater than // the current capacity. state_.value_stack().SetMaxSize(subplan.size()); + ExecutionFrame frame(subplan, empty_, context.options(), state_); auto result = frame.Evaluate(); // If this would be a runtime error, then don't adjust the program plan, but diff --git a/eval/eval/evaluator_core.cc b/eval/eval/evaluator_core.cc index 1d8e13b3f..7c963aebc 100644 --- a/eval/eval/evaluator_core.cc +++ b/eval/eval/evaluator_core.cc @@ -44,9 +44,13 @@ namespace google::api::expr::runtime { void FlatExpressionEvaluatorState::Reset() { + if (!dirty_) { + return; + } value_stack_.Clear(); iterator_stack_.Clear(); comprehension_slots_.Reset(); + dirty_ = false; } const ExpressionStep* ExecutionFrame::NextSlow() { diff --git a/eval/eval/evaluator_core.h b/eval/eval/evaluator_core.h index efb6e4461..b2b851e34 100644 --- a/eval/eval/evaluator_core.h +++ b/eval/eval/evaluator_core.h @@ -488,6 +488,7 @@ class FlatExpressionEvaluatorState { arena_(arena) {} void Reset(); + void MarkDirty() { dirty_ = true; } EvaluatorStack& value_stack() { return value_stack_; } @@ -527,6 +528,7 @@ class FlatExpressionEvaluatorState { const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool_; google::protobuf::MessageFactory* absl_nullability_unknown message_factory_ = nullptr; google::protobuf::Arena* absl_nullability_unknown arena_ = nullptr; + bool dirty_ = false; }; // Context needed for evaluation. This is sufficient for supporting @@ -728,7 +730,9 @@ class ExecutionFrame : public ExecutionFrameBase { execution_path_(flat), value_stack_(&state.value_stack()), iterator_stack_(&state.iterator_stack()), - subexpressions_(&execution_path_, 1) {} + subexpressions_(&execution_path_, 1) { + state.MarkDirty(); + } ExecutionFrame( absl::Span subexpressions, @@ -746,6 +750,7 @@ class ExecutionFrame : public ExecutionFrameBase { iterator_stack_(&state.iterator_stack()), subexpressions_(subexpressions) { ABSL_DCHECK(!subexpressions.empty()); + state.MarkDirty(); } // Returns next expression to evaluate.