From 73625f35aab4e362c85cd717137e53092c9b0f89 Mon Sep 17 00:00:00 2001 From: Justin King Date: Fri, 4 Apr 2025 12:54:08 -0700 Subject: [PATCH] Optimize `EvaluatorStack` PiperOrigin-RevId: 744041023 --- eval/eval/BUILD | 7 + eval/eval/evaluator_stack.cc | 89 +++++++++- eval/eval/evaluator_stack.h | 313 +++++++++++++++++++++++++---------- 3 files changed, 315 insertions(+), 94 deletions(-) diff --git a/eval/eval/BUILD b/eval/eval/BUILD index 02b77a3fb..f5b8f2828 100644 --- a/eval/eval/BUILD +++ b/eval/eval/BUILD @@ -136,8 +136,15 @@ cc_library( deps = [ ":attribute_trail", "//common:value", + "//internal:align", + "//internal:new", "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/base:dynamic_annotations", + "@com_google_absl//absl/base:nullability", + "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/log:absl_log", + "@com_google_absl//absl/meta:type_traits", + "@com_google_absl//absl/types:optional", "@com_google_absl//absl/types:span", ], ) diff --git a/eval/eval/evaluator_stack.cc b/eval/eval/evaluator_stack.cc index c7a62eff6..ad3340752 100644 --- a/eval/eval/evaluator_stack.cc +++ b/eval/eval/evaluator_stack.cc @@ -1,11 +1,92 @@ #include "eval/eval/evaluator_stack.h" +#include +#include +#include +#include +#include + +#include "absl/base/dynamic_annotations.h" +#include "absl/base/nullability.h" +#include "absl/log/absl_log.h" +#include "common/value.h" +#include "eval/eval/attribute_trail.h" +#include "internal/new.h" + namespace google::api::expr::runtime { -void EvaluatorStack::Clear() { - stack_.clear(); - attribute_stack_.clear(); - current_size_ = 0; +void EvaluatorStack::Grow() { + const size_t new_max_size = std::max(max_size() * 2, size_t{1}); + ABSL_LOG(ERROR) << "evaluation stack is unexpectedly full: growing from " + << max_size() << " to " << new_max_size + << " as a last resort to avoid crashing: this should not " + "have happened so there must be a bug somewhere in " + "the planner or evaluator"; + Reserve(new_max_size); +} + +void EvaluatorStack::Reserve(size_t size) { + static_assert(alignof(cel::Value) <= __STDCPP_DEFAULT_NEW_ALIGNMENT__); + static_assert(alignof(AttributeTrail) <= __STDCPP_DEFAULT_NEW_ALIGNMENT__); + + if (max_size_ >= size) { + return; + } + + absl::NullabilityUnknown data = cel::internal::New(SizeBytes(size)); + + absl::NullabilityUnknown values_begin = + reinterpret_cast(data); + absl::NullabilityUnknown values = values_begin; + + absl::NullabilityUnknown attributes_begin = + reinterpret_cast(reinterpret_cast(data) + + AttributesBytesOffset(size)); + absl::NullabilityUnknown attributes = attributes_begin; + + if (max_size_ > 0) { + const size_t n = this->size(); + const size_t m = std::min(n, size); + + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(values_begin, values_begin + size, + values_begin + size, values + m); + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(attributes_begin, + attributes_begin + size, + attributes_begin + size, attributes + m); + + for (size_t i = 0; i < m; ++i) { + ::new (static_cast(values++)) + cel::Value(std::move(values_begin_[i])); + ::new (static_cast(attributes++)) + AttributeTrail(std::move(attributes_begin_[i])); + } + std::destroy_n(values_begin_, n); + std::destroy_n(attributes_begin_, n); + + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(values_begin_, values_begin_ + max_size_, + values_, values_begin_ + max_size_); + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER( + attributes_begin_, attributes_begin_ + max_size_, attributes_, + attributes_begin_ + max_size_); + + cel::internal::SizedDelete(data_, SizeBytes(max_size_)); + } else { + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(values_begin, values_begin + size, + values_begin + size, values); + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(attributes_begin, + attributes_begin + size, + attributes_begin + size, attributes); + } + + values_ = values; + values_begin_ = values_begin; + values_end_ = values_begin + size; + + attributes_ = attributes; + attributes_begin_ = attributes_begin; + + data_ = data; + max_size_ = size; } } // namespace google::api::expr::runtime diff --git a/eval/eval/evaluator_stack.h b/eval/eval/evaluator_stack.h index 487a3e5a0..e85d72e50 100644 --- a/eval/eval/evaluator_stack.h +++ b/eval/eval/evaluator_stack.h @@ -1,15 +1,24 @@ #ifndef THIRD_PARTY_CEL_CPP_EVAL_EVAL_EVALUATOR_STACK_H_ #define THIRD_PARTY_CEL_CPP_EVAL_EVAL_EVALUATOR_STACK_H_ +#include #include +#include +#include #include -#include +#include "absl/base/attributes.h" +#include "absl/base/dynamic_annotations.h" +#include "absl/base/nullability.h" #include "absl/base/optimization.h" -#include "absl/log/absl_log.h" +#include "absl/log/absl_check.h" +#include "absl/meta/type_traits.h" +#include "absl/types/optional.h" #include "absl/types/span.h" #include "common/value.h" #include "eval/eval/attribute_trail.h" +#include "internal/align.h" +#include "internal/new.h" namespace google::api::expr::runtime { @@ -18,142 +27,260 @@ namespace google::api::expr::runtime { // stack as Span<>. class EvaluatorStack { public: - explicit EvaluatorStack(size_t max_size) - : max_size_(max_size), current_size_(0) { - Reserve(max_size); + explicit EvaluatorStack(size_t max_size) { Reserve(max_size); } + + EvaluatorStack(const EvaluatorStack&) = delete; + EvaluatorStack(EvaluatorStack&&) = delete; + + ~EvaluatorStack() { + if (max_size() > 0) { + const size_t n = size(); + std::destroy_n(values_begin_, n); + std::destroy_n(attributes_begin_, n); + cel::internal::SizedDelete(data_, SizeBytes(max_size_)); + } } + EvaluatorStack& operator=(const EvaluatorStack&) = delete; + EvaluatorStack& operator=(EvaluatorStack&&) = delete; + // Return the current stack size. - size_t size() const { return current_size_; } + size_t size() const { + ABSL_DCHECK_GE(values_, values_begin_); + ABSL_DCHECK_LE(values_, values_begin_ + max_size_); + ABSL_DCHECK_GE(attributes_, attributes_begin_); + ABSL_DCHECK_LE(attributes_, attributes_begin_ + max_size_); + ABSL_DCHECK_EQ(values_ - values_begin_, attributes_ - attributes_begin_); + + return values_ - values_begin_; + } // Return the maximum size of the stack. - size_t max_size() const { return max_size_; } + size_t max_size() const { + ABSL_DCHECK_GE(values_, values_begin_); + ABSL_DCHECK_LE(values_, values_begin_ + max_size_); + ABSL_DCHECK_GE(attributes_, attributes_begin_); + ABSL_DCHECK_LE(attributes_, attributes_begin_ + max_size_); + ABSL_DCHECK_EQ(values_ - values_begin_, attributes_ - attributes_begin_); + + return max_size_; + } // Returns true if stack is empty. - bool empty() const { return current_size_ == 0; } + bool empty() const { + ABSL_DCHECK_GE(values_, values_begin_); + ABSL_DCHECK_LE(values_, values_begin_ + max_size_); + ABSL_DCHECK_GE(attributes_, attributes_begin_); + ABSL_DCHECK_LE(attributes_, attributes_begin_ + max_size_); + ABSL_DCHECK_EQ(values_ - values_begin_, attributes_ - attributes_begin_); + + return values_ == values_begin_; + } + + bool full() const { + ABSL_DCHECK_GE(values_, values_begin_); + ABSL_DCHECK_LE(values_, values_begin_ + max_size_); + ABSL_DCHECK_GE(attributes_, attributes_begin_); + ABSL_DCHECK_LE(attributes_, attributes_begin_ + max_size_); + ABSL_DCHECK_EQ(values_ - values_begin_, attributes_ - attributes_begin_); + + return values_ == values_end_; + } // Attributes stack size. - size_t attribute_size() const { return current_size_; } + ABSL_DEPRECATED("Use size()") + size_t attribute_size() const { return size(); } // Check that stack has enough elements. - bool HasEnough(size_t size) const { return current_size_ >= size; } + bool HasEnough(size_t size) const { return this->size() >= size; } // Dumps the entire stack state as is. - void Clear(); + void Clear() { + if (max_size() > 0) { + const size_t n = size(); + std::destroy_n(values_begin_, n); + std::destroy_n(attributes_begin_, n); + + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER( + values_begin_, values_begin_ + max_size_, values_, values_begin_); + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(attributes_begin_, + attributes_begin_ + max_size_, + attributes_, attributes_begin_); + + values_ = values_begin_; + attributes_ = attributes_begin_; + } + } // Gets the last size elements of the stack. // Checking that stack has enough elements is caller's responsibility. // Please note that calls to Push may invalidate returned Span object. absl::Span GetSpan(size_t size) const { - if (ABSL_PREDICT_FALSE(!HasEnough(size))) { - ABSL_LOG(FATAL) << "Requested span size (" << size - << ") exceeds current stack size: " << current_size_; - } - return absl::Span(stack_.data() + current_size_ - size, - size); + ABSL_DCHECK(HasEnough(size)); + + return absl::Span(values_ - size, size); } // Gets the last size attribute trails of the stack. // Checking that stack has enough elements is caller's responsibility. // Please note that calls to Push may invalidate returned Span object. absl::Span GetAttributeSpan(size_t size) const { - if (ABSL_PREDICT_FALSE(!HasEnough(size))) { - ABSL_LOG(FATAL) << "Requested span size (" << size - << ") exceeds current stack size: " << current_size_; - } - return absl::Span( - attribute_stack_.data() + current_size_ - size, size); + ABSL_DCHECK(HasEnough(size)); + + return absl::Span(attributes_ - size, size); } // Peeks the last element of the stack. // Checking that stack is not empty is caller's responsibility. cel::Value& Peek() { - if (ABSL_PREDICT_FALSE(empty())) { - ABSL_LOG(FATAL) << "Peeking on empty EvaluatorStack"; - } - return stack_[current_size_ - 1]; + ABSL_DCHECK(HasEnough(1)); + + return *(values_ - 1); } // Peeks the last element of the stack. // Checking that stack is not empty is caller's responsibility. const cel::Value& Peek() const { - if (ABSL_PREDICT_FALSE(empty())) { - ABSL_LOG(FATAL) << "Peeking on empty EvaluatorStack"; - } - return stack_[current_size_ - 1]; + ABSL_DCHECK(HasEnough(1)); + + return *(values_ - 1); } // Peeks the last element of the attribute stack. // Checking that stack is not empty is caller's responsibility. const AttributeTrail& PeekAttribute() const { - if (ABSL_PREDICT_FALSE(empty())) { - ABSL_LOG(FATAL) << "Peeking on empty EvaluatorStack"; - } - return attribute_stack_[current_size_ - 1]; + ABSL_DCHECK(HasEnough(1)); + + return *(attributes_ - 1); } // Peeks the last element of the attribute stack. // Checking that stack is not empty is caller's responsibility. AttributeTrail& PeekAttribute() { - if (ABSL_PREDICT_FALSE(empty())) { - ABSL_LOG(FATAL) << "Peeking on empty EvaluatorStack"; - } - return attribute_stack_[current_size_ - 1]; + ABSL_DCHECK(HasEnough(1)); + + return *(attributes_ - 1); + } + + void Pop() { + ABSL_DCHECK(!empty()); + + --values_; + values_->~Value(); + --attributes_; + attributes_->~AttributeTrail(); + + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(values_begin_, values_begin_ + max_size_, + values_ + 1, values_); + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(attributes_begin_, + attributes_begin_ + max_size_, + attributes_ + 1, attributes_); } // Clears the last size elements of the stack. // Checking that stack has enough elements is caller's responsibility. void Pop(size_t size) { - if (ABSL_PREDICT_FALSE(!HasEnough(size))) { - ABSL_LOG(FATAL) << "Trying to pop more elements (" << size - << ") than the current stack size: " << current_size_; - } - while (size > 0) { - stack_.pop_back(); - attribute_stack_.pop_back(); - current_size_--; - size--; + ABSL_DCHECK(HasEnough(size)); + + for (; size > 0; --size) { + Pop(); } } - // Put element on the top of the stack. - void Push(cel::Value value) { Push(std::move(value), AttributeTrail()); } + template , + std::is_convertible>>> + void Push(V&& value, A&& attribute) { + ABSL_DCHECK(!full()); - void Push(cel::Value value, AttributeTrail attribute) { - if (ABSL_PREDICT_FALSE(current_size_ >= max_size())) { - ABSL_LOG(ERROR) << "No room to push more elements on to EvaluatorStack"; + if (ABSL_PREDICT_FALSE(full())) { + Grow(); } - stack_.push_back(std::move(value)); - attribute_stack_.push_back(std::move(attribute)); - current_size_++; + + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(values_begin_, values_begin_ + max_size_, + values_, values_ + 1); + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(attributes_begin_, + attributes_begin_ + max_size_, + attributes_, attributes_ + 1); + + ::new (static_cast(values_++)) cel::Value(std::forward(value)); + ::new (static_cast(attributes_++)) + AttributeTrail(std::forward(attribute)); } - void PopAndPush(size_t size, cel::Value value, AttributeTrail attribute) { - if (size == 0) { - Push(std::move(value), std::move(attribute)); - return; - } - Pop(size - 1); - stack_[current_size_ - 1] = std::move(value); - attribute_stack_[current_size_ - 1] = std::move(attribute); + template >> + void Push(V&& value) { + ABSL_DCHECK(!full()); + + Push(std::forward(value), absl::nullopt); } - // Replace element on the top of the stack. - // Checking that stack is not empty is caller's responsibility. - void PopAndPush(cel::Value value) { - PopAndPush(std::move(value), AttributeTrail()); + // Equivalent to `PopAndPush(1, ...)`. + template , + std::is_convertible>>> + void PopAndPush(V&& value, A&& attribute) { + ABSL_DCHECK(!empty()); + + *(values_ - 1) = std::forward(value); + *(attributes_ - 1) = std::forward(attribute); } - // Replace element on the top of the stack. - // Checking that stack is not empty is caller's responsibility. - void PopAndPush(cel::Value value, AttributeTrail attribute) { - PopAndPush(1, std::move(value), std::move(attribute)); + // Equivalent to `PopAndPush(1, ...)`. + template >> + void PopAndPush(V&& value) { + ABSL_DCHECK(!empty()); + + PopAndPush(std::forward(value), absl::nullopt); + } + + // Equivalent to `Pop(n)` followed by `Push(...)`. Both `V` and `A` MUST NOT + // be located on the stack. If this is the case, use SwapAndPop instead. + template , + std::is_convertible>>> + void PopAndPush(size_t n, V&& value, A&& attribute) { + if (n > 0) { + if constexpr (std::is_same_v>) { + ABSL_DCHECK(&value < values_begin_ || + &value >= values_begin_ + max_size_) + << "Attmpting to push a value about to be popped, use PopAndSwap " + "instead."; + } + if constexpr (std::is_same_v>) { + ABSL_DCHECK(&attribute < attributes_begin_ || + &attribute >= attributes_begin_ + max_size_) + << "Attmpting to push an attribute about to be popped, use " + "PopAndSwap instead."; + } + + Pop(n - 1); + + ABSL_DCHECK(!empty()); + + *(values_ - 1) = std::forward(value); + *(attributes_ - 1) = std::forward(attribute); + } else { + Push(std::forward(value), std::forward(attribute)); + } } - void PopAndPush(size_t size, cel::Value value) { - PopAndPush(size, std::move(value), AttributeTrail{}); + // Equivalent to `Pop(n)` followed by `Push(...)`. `V` MUST NOT be located on + // the stack. If this is the case, use SwapAndPop instead. + template >> + void PopAndPush(size_t n, V&& value) { + PopAndPush(n, std::forward(value), absl::nullopt); } + // Swaps the `n - i` element (from the top of the stack) with the `n` element, + // and pops `n - 1` elements. This results in the `n - i` element being at the + // top of the stack. void SwapAndPop(size_t n, size_t i) { ABSL_DCHECK_GT(n, 0); ABSL_DCHECK_LT(i, n); @@ -162,31 +289,37 @@ class EvaluatorStack { using std::swap; if (i > 0) { - cel::Value* values = stack_.data() + current_size_; - AttributeTrail* attributes = attribute_stack_.data() + current_size_; - swap(*(values - n), *(values - n + i)); - swap(*(attributes - n), *(attributes - n + i)); + swap(*(values_ - n), *(values_ - n + i)); + swap(*(attributes_ - n), *(attributes_ - n + i)); } Pop(n - 1); } // Update the max size of the stack and update capacity if needed. - void SetMaxSize(size_t size) { - max_size_ = size; - Reserve(size); - } + void SetMaxSize(size_t size) { Reserve(size); } private: - // Preallocate stack. - void Reserve(size_t size) { - stack_.reserve(size); - attribute_stack_.reserve(size); + static size_t AttributesBytesOffset(size_t size) { + return cel::internal::AlignUp(sizeof(cel::Value) * size, + __STDCPP_DEFAULT_NEW_ALIGNMENT__); + } + + static size_t SizeBytes(size_t size) { + return AttributesBytesOffset(size) + (sizeof(AttributeTrail) * size); } - std::vector stack_; - std::vector attribute_stack_; - size_t max_size_; - size_t current_size_; + void Grow(); + + // Preallocate stack. + void Reserve(size_t size); + + absl::NullabilityUnknown values_ = nullptr; + absl::NullabilityUnknown values_begin_ = nullptr; + absl::NullabilityUnknown attributes_ = nullptr; + absl::NullabilityUnknown attributes_begin_ = nullptr; + absl::NullabilityUnknown values_end_ = nullptr; + absl::NullabilityUnknown data_ = nullptr; + size_t max_size_ = 0; }; } // namespace google::api::expr::runtime