From 68a6d01a9d67f2364e979740c6f97c4baf827a44 Mon Sep 17 00:00:00 2001 From: Jonathan Tatum Date: Thu, 1 Oct 2026 16:27:21 -0700 Subject: [PATCH] Tune inlining for core interpreter loop. PiperOrigin-RevId: 991971585 --- eval/eval/equality_steps.h | 3 +- eval/eval/evaluator_core.cc | 232 +++++++---------------------------- eval/eval/evaluator_core.h | 233 ++++++++++++++++++++++++++++++++---- 3 files changed, 253 insertions(+), 215 deletions(-) diff --git a/eval/eval/equality_steps.h b/eval/eval/equality_steps.h index ef2c4bd20..141e571d1 100644 --- a/eval/eval/equality_steps.h +++ b/eval/eval/equality_steps.h @@ -19,10 +19,11 @@ #include #include "eval/eval/direct_expression_step.h" -#include "eval/eval/evaluator_core.h" namespace google::api::expr::runtime { +class ExecutionFrame; + // Factory method for recursive _==_/_!=_ Execution step std::unique_ptr CreateDirectEqualityStep( std::unique_ptr lhs, diff --git a/eval/eval/evaluator_core.cc b/eval/eval/evaluator_core.cc index 77f0bb30f..b34c09c7c 100644 --- a/eval/eval/evaluator_core.cc +++ b/eval/eval/evaluator_core.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "absl/base/attributes.h" @@ -28,6 +29,7 @@ #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" +#include "absl/types/span.h" #include "common/value.h" #include "common/value_kind.h" #include "common/values/list_value_builder.h" @@ -52,7 +54,7 @@ void FlatExpressionEvaluatorState::Reset() { comprehension_slots_.Reset(); } -const ExpressionStep* ExecutionFrame::Next() { +const ExpressionStep* ExecutionFrame::NextSlow() { while (true) { const size_t end_pos = execution_path_.size(); @@ -79,6 +81,13 @@ const ExpressionStep* ExecutionFrame::Next() { } } +void ExecutionFrame::AbortJumpOutOfRange(int offset) { + Abort(absl::Status( + absl::StatusCode::kInternal, + absl::StrCat("Jump address out of range: position: ", pc_, + ", offset: ", offset, ", range: ", execution_path_.size()))); +} + namespace { // This class abuses the fact that `absl::Status` is trivially destructible when @@ -114,64 +123,9 @@ class EvaluationStatus final { alignas(absl::Status) char status_[sizeof(absl::Status)]; }; -void EvaluateReadSlotStep(size_t slot_index, ExecutionFrame& frame) { - const ComprehensionSlots::Slot* slot = - frame.comprehension_slots().Get(slot_index); - if (!slot->Has()) { - frame.Abort(absl::InternalError(absl::StrCat( - "Comprehension variable read out of scope: ", slot_index))); - return; - } - frame.value_stack().Push(slot->value(), slot->attribute()); -} - -void EvaluateBoolJumpStep(const BoolJumpStepInfo& step, bool target, - ExecutionFrame& frame) { - ABSL_DCHECK(step.set) << "BoolJumpStep did not have a value set."; - if (!frame.value_stack().HasEnough(step.arg_count)) { - frame.Abort( - absl::Status(absl::StatusCode::kInternal, "Value stack underflow")); - return; - } - const cel::Value& value = frame.value_stack().Peek(); - if (value.IsBool() && value.GetBool().NativeValue() == target) { - frame.value_stack().SwapAndPop(step.arg_count, step.arg_count - 1); - frame.JumpToOrAbort(step.offset); - } - // No-op if the value is not a bool or the value is not the target. - // Cleanup will happen if we hit a later jump or we fall-through. -} - -void EvaluateTernaryJumpStep(const TernaryJumpStepInfo& step, - ExecutionFrame& frame) { - ABSL_DCHECK(step.set) << "TernaryJumpStep did not have a value set."; - if (!frame.value_stack().HasEnough(1)) { - frame.Abort(absl::InternalError("TernaryJumpStep: value stack underflow")); - return; - } - const cel::Value& condition = frame.value_stack().Peek(); - switch (condition.kind()) { - case cel::ValueKind::kBool: - if (!condition.GetBool().NativeValue()) { - frame.JumpToOrAbort(step.jump_to_second_offset); - } - frame.value_stack().Pop(); - break; - default: - frame.value_stack().PopAndPush(cel::ErrorValue::From( - cel::runtime_internal::CreateNoMatchingOverloadError( - ""), - frame.arena())); - ABSL_FALLTHROUGH_INTENDED; - case cel::ValueKind::kError: - case cel::ValueKind::kUnknown: - // Propagate the error or unknown set. - frame.JumpToOrAbort(step.error_offset); - break; - } -} +} // namespace -void EvaluateMutableListAppendStep(ExecutionFrame& frame) { +void ExpressionStep::EvaluateMutableListAppendStep(ExecutionFrame& frame) { if (!frame.value_stack().HasEnough(2)) { frame.Abort( absl::Status(absl::StatusCode::kInternal, "Value stack underflow")); @@ -187,7 +141,7 @@ void EvaluateMutableListAppendStep(ExecutionFrame& frame) { return; } if (frame.unknown_processing_enabled()) { - absl::optional unknown_set = + std::optional unknown_set = frame.attribute_utility().IdentifyAndMergeUnknowns( args, frame.value_stack().GetAttributeSpan(2), /*use_partial=*/true); @@ -211,114 +165,33 @@ void EvaluateMutableListAppendStep(ExecutionFrame& frame) { absl::InvalidArgumentError("Unexpected call to runtime list append.")); } -} // namespace +void ExecutionFrame::Interpret() { + for (const ExpressionStep* expr = Next(); expr != nullptr; expr = Next()) { + expr->Evaluate(*this); + } +} -void ExpressionStep::Evaluate(ExecutionFrame* context) const { - switch (header_.kind) { - case ExpressionStepKind::kGenericLogic: - u_.logic->Evaluate(context); - break; - case ExpressionStepKind::kIntConstant: - context->value_stack().Push(cel::IntValue(u_.int_val)); - break; - case ExpressionStepKind::kBoolConstant: - context->value_stack().Push(cel::BoolValue(u_.bool_val)); - break; - case ExpressionStepKind::kDoubleConstant: - context->value_stack().Push(cel::DoubleValue(u_.double_val)); - break; - case ExpressionStepKind::kNullConstant: - context->value_stack().Push(cel::NullValue()); - break; - case ExpressionStepKind::kUintConstant: - context->value_stack().Push(cel::UintValue(u_.uint_val)); - break; - case ExpressionStepKind::kOtherConstant: - context->value_stack().Push(*u_.other_val); - break; - case ExpressionStepKind::kLazyInit: - EvaluateLazyInitStep(u_.lazy_init, *context); - break; - case ExpressionStepKind::kAssignSlotAndPop: - EvaluateAssignSlotAndPop(u_.slot_index, *context); - break; - case ExpressionStepKind::kClearSlots: - EvaluateClearSlotStep(u_.clear_slots, *context); - break; - case ExpressionStepKind::kBooleanNot: - EvaluateNotStep(*context); - break; - case ExpressionStepKind::kNotStrictlyFalse: - EvaluateNotStrictlyFalseStep(*context); - break; - case ExpressionStepKind::kBooleanOr: - EvaluateBoolLogicStep(BoolLogicKind::kOr, u_.arg_count, *context); - break; - case ExpressionStepKind::kBooleanAnd: - EvaluateBoolLogicStep(BoolLogicKind::kAnd, u_.arg_count, *context); - break; - case ExpressionStepKind::kComprehensionFinish: - EvaluateComprehensionFinishStep(u_.slot_index, *context); - break; - case ExpressionStepKind::kComprehensionNext: - u_.next_step.Evaluate1(context); - break; - case ExpressionStepKind::kComprehensionNext2: - u_.next_step.Evaluate2(context); - break; - case ExpressionStepKind::kComprehensionCond: - u_.cond_step.Evaluate1(context); - break; - case ExpressionStepKind::kComprehensionCond2: - u_.cond_step.Evaluate2(context); - break; - case ExpressionStepKind::kReadSlot: - EvaluateReadSlotStep(u_.slot_index, *context); - break; - case ExpressionStepKind::kBooleanOrJump: - EvaluateBoolJumpStep(u_.bool_jump_step, true, *context); - break; - case ExpressionStepKind::kBooleanAndJump: - EvaluateBoolJumpStep(u_.bool_jump_step, false, *context); - break; - case ExpressionStepKind::kTernaryJump: - EvaluateTernaryJumpStep(u_.ternary_jump_step, *context); - break; - case ExpressionStepKind::kFixedJump: - ABSL_DCHECK(u_.fixed_jump_step.set) - << "FixedJumpStep did not have a value set."; - context->JumpToOrAbort(u_.fixed_jump_step.offset); - break; - case ExpressionStepKind::kEagerFunction: - u_.eager_function_step->Evaluate(*context); - break; - case ExpressionStepKind::kLazyFunction: - u_.lazy_function_step->Evaluate(*context); - break; - case ExpressionStepKind::kFastIn: - EvaluateFastInStep(*context); - break; - case ExpressionStepKind::kFastEqual: - EvaluateFastEqualStep(/*negation=*/false, *context); - break; - case ExpressionStepKind::kFastNotEqual: - EvaluateFastEqualStep(/*negation=*/true, *context); - break; - case ExpressionStepKind::kNewMutableList: - context->value_stack().Push(cel::CustomListValue( - cel::common_internal::NewMutableListValue(context->arena()), - context->arena())); - break; - case ExpressionStepKind::kMutableListAppend: - EvaluateMutableListAppendStep(*context); - break; - case ExpressionStepKind::kMovedFrom: - context->Abort( - absl::InternalError("ExpressionStep::Evaluate called on moved-from " - "object")); - break; - default: - ABSL_UNREACHABLE(); +void ExecutionFrame::InterpretWithCallback(EvaluationListener& listener) { + for (const ExpressionStep* expr = Next(); expr != nullptr; expr = Next()) { + expr->Evaluate(*this); + if (pc_ == 0 || !expr->comes_from_ast() || !abort_status().ok()) { + // Skip if we just started a Call, if the step doesn't map to an AST + // id, or if evaluation was aborted. + continue; + } + + if (ABSL_PREDICT_FALSE(value_stack().empty())) { + ABSL_LOG(ERROR) << "Stack is empty after a ExpressionStep.Evaluate. " + "Try to disable short-circuiting."; + continue; + } + if (EvaluationStatus status(listener(expr->id(), value_stack().Peek(), + descriptor_pool(), message_factory(), + arena())); + !status.ok()) { + Abort(std::move(status).Consume()); + return; + } } } @@ -327,32 +200,9 @@ absl::StatusOr ExecutionFrame::Evaluate( const size_t initial_stack_size = value_stack().size(); if (!listener) { - for (const ExpressionStep* expr = Next(); - ABSL_PREDICT_TRUE(expr != nullptr); expr = Next()) { - expr->Evaluate(this); - } + Interpret(); } else { - for (const ExpressionStep* expr = Next(); - ABSL_PREDICT_TRUE(expr != nullptr); expr = Next()) { - expr->Evaluate(this); - if (pc_ == 0 || !expr->comes_from_ast() || !abort_status().ok()) { - // Skip if we just started a Call or if the step doesn't map to an - // AST id. - continue; - } - - if (ABSL_PREDICT_FALSE(value_stack().empty())) { - ABSL_LOG(ERROR) << "Stack is empty after a ExpressionStep.Evaluate. " - "Try to disable short-circuiting."; - continue; - } - if (EvaluationStatus status(listener(expr->id(), value_stack().Peek(), - descriptor_pool(), message_factory(), - arena())); - !status.ok()) { - return std::move(status).Consume(); - } - } + InterpretWithCallback(listener); } if (!abort_status().ok()) { diff --git a/eval/eval/evaluator_core.h b/eval/eval/evaluator_core.h index 8e71fcd83..f6137689c 100644 --- a/eval/eval/evaluator_core.h +++ b/eval/eval/evaluator_core.h @@ -22,6 +22,7 @@ #include #include +#include "absl/base/attributes.h" #include "absl/base/nullability.h" #include "absl/base/optimization.h" #include "absl/log/absl_check.h" @@ -33,10 +34,13 @@ #include "base/type_provider.h" #include "common/native_type.h" #include "common/value.h" +#include "common/value_kind.h" +#include "common/values/list_value_builder.h" #include "eval/eval/attribute_utility.h" #include "eval/eval/comprehension_slots.h" #include "eval/eval/comprehension_step.h" #include "eval/eval/direct_expression_step.h" +#include "eval/eval/equality_steps.h" #include "eval/eval/evaluator_stack.h" #include "eval/eval/expression_step_logic.h" #include "eval/eval/function_step.h" @@ -45,6 +49,7 @@ #include "eval/eval/logic_step.h" #include "runtime/activation_interface.h" #include "runtime/internal/activation_attribute_matcher_access.h" +#include "runtime/internal/errors.h" #include "runtime/runtime.h" #include "runtime/runtime_options.h" #include "google/protobuf/arena.h" @@ -148,7 +153,9 @@ class ExpressionStep { // Returns if the execution step comes from AST. bool comes_from_ast() const { return header_.id >= 0; } - void Evaluate(ExecutionFrame* context) const; + // Evaluates this step on the given execution frame. + ABSL_ATTRIBUTE_ALWAYS_INLINE inline void Evaluate( + ExecutionFrame& frame) const; const ExpressionStepLogic* GetGenericStep() const; bool IsGenericStep() const; @@ -316,6 +323,14 @@ class ExpressionStep { } private: + static ABSL_ATTRIBUTE_ALWAYS_INLINE inline void EvaluateReadSlotStep( + size_t slot_index, ExecutionFrame& frame); + static ABSL_ATTRIBUTE_ALWAYS_INLINE inline void EvaluateBoolJumpStep( + const BoolJumpStepInfo& step, bool target, ExecutionFrame& frame); + static ABSL_ATTRIBUTE_ALWAYS_INLINE inline void EvaluateTernaryJumpStep( + const TernaryJumpStepInfo& step, ExecutionFrame& frame); + static void EvaluateMutableListAppendStep(ExecutionFrame& frame); + struct Header { ExpressionStepKind kind; uint16_t reserved; @@ -686,7 +701,14 @@ class ExecutionFrame : public ExecutionFrameBase { } // Returns next expression to evaluate. - const ExpressionStep* Next(); + ABSL_ATTRIBUTE_ALWAYS_INLINE const ExpressionStep* Next() { + if (ABSL_PREDICT_TRUE(pc_ < execution_path_.size())) { + const ExpressionStep* step = &execution_path_[pc_++]; + ABSL_ASSUME(step != nullptr); + return step; + } + return NextSlow(); + } // Evaluate the execution frame to completion. absl::StatusOr Evaluate(EvaluationListener& listener); @@ -697,31 +719,14 @@ class ExecutionFrame : public ExecutionFrameBase { // // Offset applies after normal pc increment. For example, JumpTo(0) is a // no-op, JumpTo(1) skips the expected next step. - absl::Status JumpTo(int offset) { - ABSL_DCHECK_LE(offset, static_cast(execution_path_.size())); - ABSL_DCHECK_GE(offset, -static_cast(pc_)); - - int new_pc = static_cast(pc_) + offset; - if (new_pc < 0 || new_pc > static_cast(execution_path_.size())) { - return absl::Status(absl::StatusCode::kInternal, - absl::StrCat("Jump address out of range: position: ", - pc_, ", offset: ", offset, - ", range: ", execution_path_.size())); - } - pc_ = static_cast(new_pc); - return absl::OkStatus(); - } - - void JumpToOrAbort(int offset) { + ABSL_ATTRIBUTE_ALWAYS_INLINE void JumpToOrAbort(int offset) { ABSL_DCHECK_LE(offset, static_cast(execution_path_.size())); ABSL_DCHECK_GE(offset, -static_cast(pc_)); int new_pc = static_cast(pc_) + offset; - if (new_pc < 0 || new_pc > static_cast(execution_path_.size())) { - Abort(absl::Status(absl::StatusCode::kInternal, - absl::StrCat("Jump address out of range: position: ", - pc_, ", offset: ", offset, - ", range: ", execution_path_.size()))); + if (ABSL_PREDICT_FALSE(new_pc < 0 || + new_pc > static_cast(execution_path_.size()))) { + AbortJumpOutOfRange(offset); return; } pc_ = static_cast(new_pc); @@ -797,6 +802,21 @@ class ExecutionFrame : public ExecutionFrameBase { size_t expected_stack_size; }; + // Runs steps until the program completes or is aborted. + void Interpret(); + + // Same as Interpret(), but also calls `listener` with the result of each + // step that maps to an AST node. + void InterpretWithCallback(EvaluationListener& listener); + + // Slow path of Next(): returns from a completed subexpression, or returns + // nullptr at the end of the program. + ABSL_ATTRIBUTE_NOINLINE const ExpressionStep* NextSlow(); + + // Error path of JumpToOrAbort(). + ABSL_ATTRIBUTE_NOINLINE ABSL_ATTRIBUTE_COLD void AbortJumpOutOfRange( + int offset); + size_t pc_; // pc_ - Program Counter. Current position on execution path. ExecutionPathView execution_path_; EvaluatorStack* absl_nonnull const value_stack_; @@ -994,6 +1014,173 @@ inline ExpressionStep& ExpressionStep::operator=(ExpressionStep&& other) { return *this; } +inline void ExpressionStep::EvaluateReadSlotStep(size_t slot_index, + ExecutionFrame& frame) { + const ComprehensionSlots::Slot* slot = + frame.comprehension_slots().Get(slot_index); + if (!slot->Has()) { + frame.Abort(absl::InternalError(absl::StrCat( + "Comprehension variable read out of scope: ", slot_index))); + return; + } + frame.value_stack().Push(slot->value(), slot->attribute()); +} + +inline void ExpressionStep::EvaluateBoolJumpStep(const BoolJumpStepInfo& step, + bool target, + ExecutionFrame& frame) { + ABSL_DCHECK(step.set) << "BoolJumpStep did not have a value set."; + if (!frame.value_stack().HasEnough(step.arg_count)) { + frame.Abort( + absl::Status(absl::StatusCode::kInternal, "Value stack underflow")); + return; + } + const cel::Value& value = frame.value_stack().Peek(); + if (value.IsBool() && value.GetBool().NativeValue() == target) { + frame.value_stack().SwapAndPop(step.arg_count, step.arg_count - 1); + frame.JumpToOrAbort(step.offset); + } + // No-op if the value is not a bool or the value is not the target. + // Cleanup will happen if we hit a later jump or we fall-through. +} + +inline void ExpressionStep::EvaluateTernaryJumpStep( + const TernaryJumpStepInfo& step, ExecutionFrame& frame) { + ABSL_DCHECK(step.set) << "TernaryJumpStep did not have a value set."; + if (!frame.value_stack().HasEnough(1)) { + frame.Abort(absl::InternalError("TernaryJumpStep: value stack underflow")); + return; + } + const cel::Value& condition = frame.value_stack().Peek(); + switch (condition.kind()) { + case cel::ValueKind::kBool: + if (!condition.GetBool().NativeValue()) { + frame.JumpToOrAbort(step.jump_to_second_offset); + } + frame.value_stack().Pop(); + break; + default: + frame.value_stack().PopAndPush(cel::ErrorValue::From( + cel::runtime_internal::CreateNoMatchingOverloadError( + ""), + frame.arena())); + ABSL_FALLTHROUGH_INTENDED; + case cel::ValueKind::kError: + case cel::ValueKind::kUnknown: + // Propagate the error or unknown set. + frame.JumpToOrAbort(step.error_offset); + break; + } +} + +inline void ExpressionStep::Evaluate(ExecutionFrame& frame) const { + switch (header_.kind) { + case ExpressionStepKind::kGenericLogic: + u_.logic->Evaluate(&frame); + break; + case ExpressionStepKind::kIntConstant: + frame.value_stack().Push(cel::IntValue(u_.int_val)); + break; + case ExpressionStepKind::kBoolConstant: + frame.value_stack().Push(cel::BoolValue(u_.bool_val)); + break; + case ExpressionStepKind::kDoubleConstant: + frame.value_stack().Push(cel::DoubleValue(u_.double_val)); + break; + case ExpressionStepKind::kNullConstant: + frame.value_stack().Push(cel::NullValue()); + break; + case ExpressionStepKind::kUintConstant: + frame.value_stack().Push(cel::UintValue(u_.uint_val)); + break; + case ExpressionStepKind::kOtherConstant: + frame.value_stack().Push(*u_.other_val); + break; + case ExpressionStepKind::kLazyInit: + EvaluateLazyInitStep(u_.lazy_init, frame); + break; + case ExpressionStepKind::kAssignSlotAndPop: + EvaluateAssignSlotAndPop(u_.slot_index, frame); + break; + case ExpressionStepKind::kClearSlots: + EvaluateClearSlotStep(u_.clear_slots, frame); + break; + case ExpressionStepKind::kBooleanNot: + EvaluateNotStep(frame); + break; + case ExpressionStepKind::kNotStrictlyFalse: + EvaluateNotStrictlyFalseStep(frame); + break; + case ExpressionStepKind::kBooleanOr: + EvaluateBoolLogicStep(BoolLogicKind::kOr, u_.arg_count, frame); + break; + case ExpressionStepKind::kBooleanAnd: + EvaluateBoolLogicStep(BoolLogicKind::kAnd, u_.arg_count, frame); + break; + case ExpressionStepKind::kComprehensionFinish: + EvaluateComprehensionFinishStep(u_.slot_index, frame); + break; + case ExpressionStepKind::kComprehensionNext: + u_.next_step.Evaluate1(&frame); + break; + case ExpressionStepKind::kComprehensionNext2: + u_.next_step.Evaluate2(&frame); + break; + case ExpressionStepKind::kComprehensionCond: + u_.cond_step.Evaluate1(&frame); + break; + case ExpressionStepKind::kComprehensionCond2: + u_.cond_step.Evaluate2(&frame); + break; + case ExpressionStepKind::kReadSlot: + EvaluateReadSlotStep(u_.slot_index, frame); + break; + case ExpressionStepKind::kBooleanOrJump: + EvaluateBoolJumpStep(u_.bool_jump_step, true, frame); + break; + case ExpressionStepKind::kBooleanAndJump: + EvaluateBoolJumpStep(u_.bool_jump_step, false, frame); + break; + case ExpressionStepKind::kTernaryJump: + EvaluateTernaryJumpStep(u_.ternary_jump_step, frame); + break; + case ExpressionStepKind::kFixedJump: + ABSL_DCHECK(u_.fixed_jump_step.set) + << "FixedJumpStep did not have a value set."; + frame.JumpToOrAbort(u_.fixed_jump_step.offset); + break; + case ExpressionStepKind::kEagerFunction: + u_.eager_function_step->Evaluate(frame); + break; + case ExpressionStepKind::kLazyFunction: + u_.lazy_function_step->Evaluate(frame); + break; + case ExpressionStepKind::kFastIn: + EvaluateFastInStep(frame); + break; + case ExpressionStepKind::kFastEqual: + EvaluateFastEqualStep(/*negation=*/false, frame); + break; + case ExpressionStepKind::kFastNotEqual: + EvaluateFastEqualStep(/*negation=*/true, frame); + break; + case ExpressionStepKind::kNewMutableList: + frame.value_stack().Push(cel::CustomListValue( + cel::common_internal::NewMutableListValue(frame.arena()), + frame.arena())); + break; + case ExpressionStepKind::kMutableListAppend: + EvaluateMutableListAppendStep(frame); + break; + case ExpressionStepKind::kMovedFrom: + frame.Abort(absl::InternalError( + "ExpressionStep::Evaluate called on moved-from step")); + break; + default: + ABSL_UNREACHABLE(); + } +} + } // namespace google::api::expr::runtime #endif // THIRD_PARTY_CEL_CPP_EVAL_EVAL_EVALUATOR_CORE_H_