Skip to content

Commit 76fbb08

Browse files
jnthntatumcopybara-github
authored andcommitted
Memoize enum lookup table in FlatExprBuilder. Switch to using fully qualified name only instead of precomputing all possible ways to reference.
PiperOrigin-RevId: 742475198
1 parent 6a9e920 commit 76fbb08

14 files changed

+307
-235
lines changed

eval/compiler/BUILD

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,7 @@ cc_test(
445445
"//eval/public:builtin_func_registrar",
446446
"//eval/public:cel_function",
447447
"//eval/public:cel_function_registry",
448+
"//eval/public:cel_value",
448449
"//extensions/protobuf:ast_converters",
449450
"//internal:casts",
450451
"//internal:proto_matchers",
@@ -499,7 +500,6 @@ cc_test(
499500
"//eval/testutil:test_message_cc_proto",
500501
"//internal:testing",
501502
"@com_google_absl//absl/status",
502-
"@com_google_absl//absl/types:optional",
503503
"@com_google_absl//absl/types:span",
504504
"@com_google_protobuf//:protobuf",
505505
],
@@ -542,16 +542,20 @@ cc_test(
542542
":flat_expr_builder",
543543
":flat_expr_builder_extensions",
544544
":regex_precompilation_optimization",
545+
":resolver",
545546
"//common/ast:ast_impl",
546547
"//eval/eval:evaluator_core",
547548
"//eval/public:activation",
548549
"//eval/public:builtin_func_registrar",
549550
"//eval/public:cel_expression",
551+
"//eval/public:cel_function_registry",
550552
"//eval/public:cel_options",
553+
"//eval/public:cel_type_registry",
551554
"//eval/public:cel_value",
552555
"//internal:testing",
553556
"//parser",
554557
"//runtime:runtime_issue",
558+
"//runtime:runtime_options",
555559
"//runtime/internal:issue_collector",
556560
"//runtime/internal:runtime_env",
557561
"//runtime/internal:runtime_env_testing",

eval/compiler/constant_folding_test.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include <memory>
1818
#include <utility>
19+
#include <vector>
1920

2021
#include "cel/expr/syntax.pb.h"
2122
#include "absl/base/nullability.h"
@@ -78,8 +79,7 @@ class UpdatedConstantFoldingTest : public testing::Test {
7879
type_registry_(env_->type_registry),
7980
issue_collector_(RuntimeIssue::Severity::kError),
8081
resolver_("", function_registry_, type_registry_,
81-
type_registry_.GetComposedTypeProvider(),
82-
type_registry_.resolveable_enums()) {}
82+
type_registry_.GetComposedTypeProvider()) {}
8383

8484
protected:
8585
absl::Nonnull<std::shared_ptr<RuntimeEnv>> env_;

eval/compiler/flat_expr_builder.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2467,12 +2467,17 @@ std::vector<ExecutionPathView> FlattenExpressionTable(
24672467

24682468
absl::StatusOr<FlatExpression> FlatExprBuilder::CreateExpressionImpl(
24692469
std::unique_ptr<Ast> ast, std::vector<RuntimeIssue>* issues) const {
2470+
if (absl::StartsWith(container_, ".") || absl::EndsWith(container_, ".")) {
2471+
return absl::InvalidArgumentError(
2472+
absl::StrCat("Invalid expression container: '", container_, "'"));
2473+
}
2474+
24702475
RuntimeIssue::Severity max_severity = options_.fail_on_warnings
24712476
? RuntimeIssue::Severity::kWarning
24722477
: RuntimeIssue::Severity::kError;
24732478
IssueCollector issue_collector(max_severity);
24742479
Resolver resolver(container_, function_registry_, type_registry_,
2475-
GetTypeProvider(), type_registry_.resolveable_enums(),
2480+
GetTypeProvider(),
24762481
options_.enable_qualified_type_identifiers);
24772482

24782483
std::shared_ptr<google::protobuf::Arena> arena;
@@ -2482,11 +2487,6 @@ absl::StatusOr<FlatExpression> FlatExprBuilder::CreateExpressionImpl(
24822487

24832488
auto& ast_impl = AstImpl::CastFromPublicAst(*ast);
24842489

2485-
if (absl::StartsWith(container_, ".") || absl::EndsWith(container_, ".")) {
2486-
return absl::InvalidArgumentError(
2487-
absl::StrCat("Invalid expression container: '", container_, "'"));
2488-
}
2489-
24902490
for (const std::unique_ptr<AstTransform>& transform : ast_transforms_) {
24912491
CEL_RETURN_IF_ERROR(transform->UpdateAst(extension_context, ast_impl));
24922492
}

eval/compiler/flat_expr_builder.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@
2323
#include <vector>
2424

2525
#include "absl/base/nullability.h"
26+
#include "absl/container/flat_hash_map.h"
2627
#include "absl/status/statusor.h"
2728
#include "absl/strings/string_view.h"
2829
#include "base/ast.h"
2930
#include "base/type_provider.h"
31+
#include "common/value.h"
3032
#include "eval/compiler/flat_expr_builder_extensions.h"
3133
#include "eval/eval/evaluator_core.h"
3234
#include "runtime/function_registry.h"
@@ -98,6 +100,7 @@ class FlatExprBuilder {
98100

99101
const absl::Nonnull<std::shared_ptr<const cel::runtime_internal::RuntimeEnv>>
100102
env_;
103+
101104
cel::RuntimeOptions options_;
102105
std::string container_;
103106
bool enable_optional_types_ = false;

eval/compiler/flat_expr_builder_extensions_test.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include <memory>
1717
#include <utility>
18+
#include <vector>
1819

1920
#include "absl/base/nullability.h"
2021
#include "absl/status/status.h"
@@ -62,8 +63,7 @@ class PlannerContextTest : public testing::Test {
6263
type_registry_(env_->type_registry),
6364
function_registry_(env_->function_registry),
6465
resolver_("", function_registry_, type_registry_,
65-
type_registry_.GetComposedTypeProvider(),
66-
type_registry_.resolveable_enums()),
66+
type_registry_.GetComposedTypeProvider()),
6767
issue_collector_(RuntimeIssue::Severity::kError) {}
6868

6969
protected:

0 commit comments

Comments
 (0)