Skip to content

Commit

Permalink
Overhaul propagation of arenas and message factories
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 691561398
  • Loading branch information
@jcking @copybara-github
jcking authored and copybara-github committed Oct 30, 2024
1 parent 883d727 commit 0353666
Show file tree
Hide file tree
Showing 38 changed files with 1,019 additions and 411 deletions.
13 changes: 4 additions & 9 deletions conformance/service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,7 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {

if (enable_optimizations_) {
CEL_RETURN_IF_ERROR(cel::extensions::EnableConstantFolding(
builder, constant_memory_manager_,
google::protobuf::MessageFactory::generated_factory()));
builder, google::protobuf::MessageFactory::generated_factory()));
}
CEL_RETURN_IF_ERROR(cel::EnableReferenceResolver(
builder, cel::ReferenceResolverEnabled::kAlways));
Expand Down Expand Up @@ -528,7 +527,7 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {

void Check(const conformance::v1alpha1::CheckRequest& request,
conformance::v1alpha1::CheckResponse& response) override {
auto status = DoCheck(&constant_arena_, request, response);
auto status = DoCheck(&arena_, request, response);
if (!status.ok()) {
auto* issue = response.add_issues();
issue->set_code(ToGrpcCode(status.code()));
Expand Down Expand Up @@ -614,10 +613,7 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
bool enable_optimizations)
: options_(options),
use_arena_(use_arena),
enable_optimizations_(enable_optimizations),
constant_memory_manager_(
use_arena_ ? ProtoMemoryManagerRef(&constant_arena_)
: cel::MemoryManagerRef::ReferenceCounting()) {}
enable_optimizations_(enable_optimizations) {}

static absl::Status DoCheck(
google::protobuf::Arena* arena, const conformance::v1alpha1::CheckRequest& request,
Expand Down Expand Up @@ -733,8 +729,7 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
RuntimeOptions options_;
bool use_arena_;
bool enable_optimizations_;
Arena constant_arena_;
cel::MemoryManagerRef constant_memory_manager_;
Arena arena_;
};

} // namespace
Expand Down
49 changes: 34 additions & 15 deletions eval/compiler/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ cc_library(
"//internal:casts",
"//runtime:runtime_options",
"//runtime/internal:issue_collector",
"//runtime/internal:runtime_env",
"@com_google_absl//absl/algorithm:container",
"@com_google_absl//absl/base:core_headers",
"@com_google_absl//absl/base:nullability",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/functional:any_invocable",
Expand All @@ -46,6 +48,7 @@ cc_library(
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/types:optional",
"@com_google_absl//absl/types:variant",
"@com_google_protobuf//:protobuf",
],
)

Expand All @@ -56,7 +59,6 @@ cc_test(
":flat_expr_builder_extensions",
":resolver",
"//base/ast_internal:expr",
"//common:casting",
"//common:memory",
"//common:native_type",
"//common:value",
Expand All @@ -71,8 +73,12 @@ cc_test(
"//runtime:runtime_options",
"//runtime:type_registry",
"//runtime/internal:issue_collector",
"//runtime/internal:runtime_env",
"//runtime/internal:runtime_env_testing",
"@com_google_absl//absl/base:nullability",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_protobuf//:protobuf",
],
)

Expand Down Expand Up @@ -115,28 +121,29 @@ cc_library(
"//eval/eval:shadowable_value_step",
"//eval/eval:ternary_step",
"//eval/eval:trace_step",
"//eval/public:cel_type_registry",
"//internal:status_macros",
"//runtime:function_registry",
"//runtime:runtime_issue",
"//runtime:runtime_options",
"//runtime:type_registry",
"//runtime/internal:convert_constant",
"//runtime/internal:issue_collector",
"//runtime/internal:runtime_env",
"@com_google_absl//absl/algorithm:container",
"@com_google_absl//absl/base:core_headers",
"@com_google_absl//absl/base:nullability",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/container:flat_hash_set",
"@com_google_absl//absl/container:node_hash_map",
"@com_google_absl//absl/log:absl_check",
"@com_google_absl//absl/log:absl_log",
"@com_google_absl//absl/log:check",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/types:optional",
"@com_google_absl//absl/types:span",
"@com_google_absl//absl/types:variant",
"@com_google_protobuf//:protobuf",
],
)

Expand All @@ -155,6 +162,7 @@ cc_test(
":qualified_reference_resolver",
"//base:function",
"//base:function_descriptor",
"//common:value",
"//eval/public:activation",
"//eval/public:builtin_func_registrar",
"//eval/public:cel_attribute",
Expand All @@ -174,13 +182,13 @@ cc_test(
"//eval/public/structs:protobuf_descriptor_type_provider",
"//eval/public/testing:matchers",
"//eval/testutil:test_message_cc_proto",
"//extensions/protobuf:memory_manager",
"//internal:proto_file_util",
"//internal:proto_matchers",
"//internal:status_macros",
"//internal:testing",
"//parser",
"//runtime:runtime_options",
"//runtime/internal:runtime_env_testing",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
Expand Down Expand Up @@ -213,6 +221,7 @@ cc_test(
"//internal:testing",
"//parser",
"//runtime:runtime_options",
"//runtime/internal:runtime_env_testing",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
"@com_google_cel_spec//proto/cel/expr:syntax_cc_proto",
Expand All @@ -236,14 +245,20 @@ cc_library(
"//eval/eval:direct_expression_step",
"//eval/eval:evaluator_core",
"//eval/public:cel_expression",
"//eval/public:cel_function_registry",
"//eval/public:cel_type_registry",
"//extensions/protobuf:ast_converters",
"//internal:status_macros",
"//runtime:runtime_issue",
"//runtime:runtime_options",
"//runtime/internal:runtime_env",
"@com_google_absl//absl/base:core_headers",
"@com_google_absl//absl/base:nullability",
"@com_google_absl//absl/log:absl_check",
"@com_google_absl//absl/log:check",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings:string_view",
"@com_google_cel_spec//proto/cel/expr:checked_cc_proto",
"@com_google_cel_spec//proto/cel/expr:syntax_cc_proto",
],
Expand All @@ -270,12 +285,12 @@ cc_test(
"//eval/public/structs:protobuf_descriptor_type_provider",
"//eval/public/testing:matchers",
"//extensions:bindings_ext",
"//extensions/protobuf:memory_manager",
"//internal:status_macros",
"//internal:testing",
"//parser",
"//parser:macro",
"//runtime:runtime_options",
"//runtime/internal:runtime_env_testing",
"@com_google_absl//absl/algorithm:container",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
Expand Down Expand Up @@ -303,7 +318,7 @@ cc_library(
"//base:kind",
"//base/ast_internal:ast_impl",
"//base/ast_internal:expr",
"//common:allocator",
"//common:memory",
"//common:value",
"//eval/eval:const_value_step",
"//eval/eval:evaluator_core",
Expand Down Expand Up @@ -331,8 +346,6 @@ cc_test(
"//base:ast",
"//base/ast_internal:ast_impl",
"//base/ast_internal:expr",
"//common:memory",
"//common:type",
"//common:value",
"//eval/eval:const_value_step",
"//eval/eval:create_list_step",
Expand All @@ -342,12 +355,16 @@ cc_test(
"//extensions/protobuf:memory_manager",
"//internal:status_macros",
"//internal:testing",
"//internal:testing_descriptor_pool",
"//parser",
"//runtime:function_registry",
"//runtime:runtime_issue",
"//runtime:runtime_options",
"//runtime:type_registry",
"//runtime/internal:issue_collector",
"//runtime/internal:runtime_env",
"//runtime/internal:runtime_env_testing",
"@com_google_absl//absl/base:nullability",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
Expand Down Expand Up @@ -390,14 +407,12 @@ cc_library(
hdrs = ["resolver.h"],
deps = [
"//base:kind",
"//common:memory",
"//common:type",
"//common:value",
"//internal:status_macros",
"//runtime:function_overload_reference",
"//runtime:function_registry",
"//runtime:type_registry",
"@com_google_absl//absl/base:nullability",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
Expand Down Expand Up @@ -445,17 +460,16 @@ cc_test(
],
deps = [
":cel_expression_builder_flat_impl",
":flat_expr_builder",
"//base:builtins",
"//eval/public:activation",
"//eval/public:cel_attribute",
"//eval/public:cel_builtins",
"//eval/public:cel_expression",
"//eval/public:cel_options",
"//eval/public:cel_value",
"//eval/public:unknown_attribute_set",
"//eval/public:unknown_set",
"//internal:status_macros",
"//internal:testing",
"//runtime:runtime_options",
"//runtime/internal:runtime_env_testing",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
"@com_google_protobuf//:protobuf",
Expand Down Expand Up @@ -533,6 +547,9 @@ cc_test(
"//parser",
"//runtime:runtime_issue",
"//runtime/internal:issue_collector",
"//runtime/internal:runtime_env",
"//runtime/internal:runtime_env_testing",
"@com_google_absl//absl/base:nullability",
"@com_google_absl//absl/status",
"@com_google_cel_spec//proto/cel/expr:checked_cc_proto",
"@com_google_cel_spec//proto/cel/expr:syntax_cc_proto",
Expand Down Expand Up @@ -581,7 +598,6 @@ cc_test(
":instrumentation",
":regex_precompilation_optimization",
"//base/ast_internal:ast_impl",
"//common:type",
"//common:value",
"//eval/eval:evaluator_core",
"//extensions/protobuf:ast_converters",
Expand All @@ -594,6 +610,9 @@ cc_test(
"//runtime:runtime_options",
"//runtime:standard_functions",
"//runtime:type_registry",
"//runtime/internal:runtime_env",
"//runtime/internal:runtime_env_testing",
"@com_google_absl//absl/base:nullability",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/status",
"@com_google_cel_spec//proto/cel/expr:syntax_cc_proto",
Expand Down
40 changes: 33 additions & 7 deletions eval/compiler/cel_expression_builder_flat_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,17 @@

#include "cel/expr/checked.pb.h"
#include "cel/expr/syntax.pb.h"
#include "absl/base/nullability.h"
#include "absl/log/absl_check.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/string_view.h"
#include "base/ast.h"
#include "eval/compiler/flat_expr_builder.h"
#include "eval/public/cel_expression.h"
#include "eval/public/cel_function_registry.h"
#include "eval/public/cel_type_registry.h"
#include "runtime/internal/runtime_env.h"
#include "runtime/runtime_options.h"

namespace google::api::expr::runtime {
Expand All @@ -37,13 +43,16 @@ namespace google::api::expr::runtime {
// Builds instances of CelExpressionFlatImpl.
class CelExpressionBuilderFlatImpl : public CelExpressionBuilder {
public:
explicit CelExpressionBuilderFlatImpl(const cel::RuntimeOptions& options)
: flat_expr_builder_(GetRegistry()->InternalGetRegistry(),
*GetTypeRegistry(), options) {}
CelExpressionBuilderFlatImpl(
absl::Nonnull<std::shared_ptr<cel::runtime_internal::RuntimeEnv>> env,
const cel::RuntimeOptions& options)
: env_(std::move(env)), flat_expr_builder_(env_, options) {
ABSL_DCHECK(env_->IsInitialized());
}

CelExpressionBuilderFlatImpl()
: flat_expr_builder_(GetRegistry()->InternalGetRegistry(),
*GetTypeRegistry()) {}
explicit CelExpressionBuilderFlatImpl(
absl::Nonnull<std::shared_ptr<cel::runtime_internal::RuntimeEnv>> env)
: CelExpressionBuilderFlatImpl(std::move(env), cel::RuntimeOptions()) {}

absl::StatusOr<std::unique_ptr<CelExpression>> CreateExpression(
const cel::expr::Expr* expr,
Expand All @@ -64,15 +73,32 @@ class CelExpressionBuilderFlatImpl : public CelExpressionBuilder {
FlatExprBuilder& flat_expr_builder() { return flat_expr_builder_; }

void set_container(std::string container) override {
CelExpressionBuilder::set_container(container);
flat_expr_builder_.set_container(std::move(container));
}

// CelFunction registry. Extension function should be registered with it
// prior to expression creation.
CelFunctionRegistry* GetRegistry() const override {
return &env_->legacy_function_registry;
}

// CEL Type registry. Provides a means to resolve the CEL built-in types to
// CelValue instances, and to extend the set of types and enums known to
// expressions by registering them ahead of time.
CelTypeRegistry* GetTypeRegistry() const override {
return &env_->legacy_type_registry;
}

absl::string_view container() const override {
return flat_expr_builder_.container();
}

private:
absl::StatusOr<std::unique_ptr<CelExpression>> CreateExpressionImpl(
std::unique_ptr<cel::Ast> converted_ast,
std::vector<absl::Status>* warnings) const;

absl::Nonnull<std::shared_ptr<cel::runtime_internal::RuntimeEnv>> env_;
FlatExprBuilder flat_expr_builder_;
};

Expand Down
Loading

0 comments on commit 0353666

Please sign in to comment.