Skip to content

Commit dc4e180

Browse files
jnthntatumcopybara-github
authored andcommitted
Add set membership optimization using precomputed bitmasks for small integer sets.
PiperOrigin-RevId: 1003462035
1 parent 4dc0a65 commit dc4e180

21 files changed

Lines changed: 1987 additions & 166 deletions

‎MODULE.bazel‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,12 @@ bazel_dep(
5959
)
6060
bazel_dep(
6161
name = "cel-spec",
62-
version = "0.25.2",
62+
version = "0.25.3",
6363
repo_name = "com_google_cel_spec",
6464
)
6565
git_override(
6666
module_name = "cel-spec",
67-
commit = "ba58ae5007845f3a1279b488cdeb79645ce958bb",
67+
commit = "6dccde40b346f89761ddb890abc6ca503a453134",
6868
remote = "https://github.com/cel-expr/cel-spec",
6969
)
7070

‎conformance/BUILD‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ cc_library(
5353
"//extensions:math_ext_macros",
5454
"//extensions:proto_ext",
5555
"//extensions:select_optimization",
56+
"//extensions:set_membership_optimization",
57+
"//extensions:sets_functions",
5658
"//extensions:strings",
5759
"//extensions/protobuf:enum_adapter",
5860
"//internal:status_macros",
@@ -162,6 +164,7 @@ _ALL_TESTS = [
162164
"@com_google_cel_spec//tests/simple:testdata/unknowns.textproto",
163165
"@com_google_cel_spec//tests/simple:testdata/wrappers.textproto",
164166
"@com_google_cel_spec//tests/simple:testdata/block_ext.textproto",
167+
"@com_google_cel_spec//tests/simple:testdata/sets_ext.textproto",
165168
"@com_google_cel_spec//tests/simple:testdata/type_deduction.textproto",
166169
]
167170

@@ -286,17 +289,17 @@ gen_conformance_tests(
286289
name = "conformance_legacy_select_opt",
287290
checked = True,
288291
data = _ALL_TESTS,
292+
extended_optimizations = True,
289293
modern = False,
290-
select_opt = True,
291294
skip_tests = _TESTS_TO_SKIP_LEGACY,
292295
)
293296

294297
gen_conformance_tests(
295298
name = "conformance_select_opt",
296299
checked = True,
297300
data = _ALL_TESTS,
301+
extended_optimizations = True,
298302
modern = True,
299-
select_opt = True,
300303
skip_tests = _TESTS_TO_SKIP_MODERN,
301304
)
302305

‎conformance/run.bzl‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,14 @@ def _conformance_test_name(name, pratt, optimize):
5656
],
5757
)
5858

59-
def _conformance_test_args(modern, optimize, select_opt, skip_check, dashboard, enable_variadic_logical_operators, pratt):
59+
def _conformance_test_args(modern, optimize, extended_optimizations, skip_check, dashboard, enable_variadic_logical_operators, pratt):
6060
args = []
6161
if modern:
6262
args.append("--modern")
6363
if optimize:
6464
args.append("--opt")
65-
if select_opt:
66-
args.append("--select_optimization")
65+
if extended_optimizations:
66+
args.append("--extended_optimizations")
6767
if skip_check:
6868
args.append("--skip_check")
6969
else:
@@ -78,10 +78,10 @@ def _conformance_test_args(modern, optimize, select_opt, skip_check, dashboard,
7878
args.append("--noenable_pratt_parser")
7979
return args
8080

81-
def _conformance_test(name, data, modern, optimize, select_opt, skip_check, skip_tests, tags, dashboard, enable_variadic_logical_operators, pratt):
81+
def _conformance_test(name, data, modern, optimize, extended_optimizations, skip_check, skip_tests, tags, dashboard, enable_variadic_logical_operators, pratt):
8282
cc_test(
8383
name = _conformance_test_name(name, pratt, optimize),
84-
args = _conformance_test_args(modern, optimize, select_opt, skip_check, dashboard, enable_variadic_logical_operators, pratt) + ["$(rlocationpath {})".format(test) for test in data],
84+
args = _conformance_test_args(modern, optimize, extended_optimizations, skip_check, dashboard, enable_variadic_logical_operators, pratt) + ["$(rlocationpath {})".format(test) for test in data],
8585
env = select(
8686
{
8787
"@platforms//os:windows": {"CEL_SKIP_TESTS": ",".join(skip_tests + _TESTS_TO_SKIP_WINDOWS)},
@@ -93,15 +93,15 @@ def _conformance_test(name, data, modern, optimize, select_opt, skip_check, skip
9393
tags = tags,
9494
)
9595

96-
def gen_conformance_tests(name, data, modern = False, checked = False, select_opt = False, dashboard = False, skip_tests = [], tags = [], enable_variadic_logical_operators = False):
96+
def gen_conformance_tests(name, data, modern = False, checked = False, extended_optimizations = False, dashboard = False, skip_tests = [], tags = [], enable_variadic_logical_operators = False):
9797
"""Generates conformance tests.
9898
9999
Args:
100100
name: prefix for all tests
101101
data: textproto targets describing conformance tests
102102
modern: run using modern APIs
103103
checked: whether to apply type checking
104-
select_opt: enable select optimization
104+
extended_optimizations: enable extended optimizations (select and set membership)
105105
dashboard: enable dashboard mode
106106
skip_tests: tests to skip in the format of the cel-spec test runner. See documentation
107107
in github.com/google/cel-spec/tests/simple/simple_test.go
@@ -119,7 +119,7 @@ def gen_conformance_tests(name, data, modern = False, checked = False, select_op
119119
data,
120120
modern = modern,
121121
optimize = optimize,
122-
select_opt = select_opt,
122+
extended_optimizations = extended_optimizations,
123123
skip_check = skip_check,
124124
skip_tests = _expand_tests_to_skip(skip_tests),
125125
tags = tags,

‎conformance/run.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ ABSL_FLAG(
6262
ABSL_FLAG(std::vector<std::string>, skip_tests, {}, "Tests to skip");
6363
ABSL_FLAG(bool, dashboard, false, "Dashboard mode, ignore test failures");
6464
ABSL_FLAG(bool, skip_check, true, "Skip type checking the expressions");
65-
ABSL_FLAG(bool, select_optimization, false, "Enable select optimization.");
65+
ABSL_FLAG(bool, extended_optimizations, false,
66+
"Enable extended optimizations.");
6667
ABSL_FLAG(bool, enable_variadic_logical_operators, false,
6768
"Enable parsing logical AND & OR operators as a single flat variadic "
6869
"call.");
@@ -261,7 +262,7 @@ NewConformanceServiceFromFlags() {
261262
cel_conformance::ConformanceServiceOptions{
262263
.optimize = absl::GetFlag(FLAGS_opt),
263264
.modern = absl::GetFlag(FLAGS_modern),
264-
.select_optimization = absl::GetFlag(FLAGS_select_optimization),
265+
.extended_optimizations = absl::GetFlag(FLAGS_extended_optimizations),
265266
.enable_variadic_logical_operators =
266267
absl::GetFlag(FLAGS_enable_variadic_logical_operators),
267268
.enable_pratt_parser = absl::GetFlag(FLAGS_enable_pratt_parser),

‎conformance/service.cc‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@
6767
#include "extensions/proto_ext.h"
6868
#include "extensions/protobuf/enum_adapter.h"
6969
#include "extensions/select_optimization.h"
70+
#include "extensions/set_membership_optimization.h"
71+
#include "extensions/sets_functions.h"
7072
#include "extensions/strings.h"
7173
#include "internal/status_macros.h"
7274
#include "parser/macro_registry.h"
@@ -196,6 +198,8 @@ absl::Status CheckImpl(google::protobuf::Arena* arena,
196198
builder->AddLibrary(cel::extensions::ComprehensionsV2CheckerLibrary()));
197199
CEL_RETURN_IF_ERROR(
198200
builder->AddLibrary(cel::extensions::ListsCheckerLibrary()));
201+
CEL_RETURN_IF_ERROR(
202+
builder->AddLibrary(cel::extensions::SetsCheckerLibrary()));
199203
}
200204

201205
for (const auto& decl : request.type_env()) {
@@ -245,7 +249,7 @@ absl::Status CheckImpl(google::protobuf::Arena* arena,
245249
class LegacyConformanceServiceImpl : public ConformanceServiceInterface {
246250
public:
247251
static absl::StatusOr<std::unique_ptr<LegacyConformanceServiceImpl>> Create(
248-
bool optimize, bool recursive, bool select_optimization,
252+
bool optimize, bool recursive, bool extended_optimizations,
249253
bool enable_variadic_logical_operators, bool enable_pratt_parser) {
250254
static auto* constant_arena = new Arena();
251255

@@ -293,8 +297,8 @@ class LegacyConformanceServiceImpl : public ConformanceServiceInterface {
293297
options.enable_typed_field_access = true;
294298
}
295299

296-
if (select_optimization) {
297-
std::cerr << "Enabling select optimizations" << std::endl;
300+
if (extended_optimizations) {
301+
std::cerr << "Enabling extended optimizations" << std::endl;
298302
options.enable_select_optimization = true;
299303
}
300304

@@ -323,6 +327,8 @@ class LegacyConformanceServiceImpl : public ConformanceServiceInterface {
323327
builder->GetRegistry(), options));
324328
CEL_RETURN_IF_ERROR(cel::extensions::RegisterMathExtensionFunctions(
325329
builder->GetRegistry(), options));
330+
CEL_RETURN_IF_ERROR(cel::extensions::RegisterSetsFunctions(
331+
builder->GetRegistry(), options));
326332

327333
return absl::WrapUnique(new LegacyConformanceServiceImpl(
328334
std::move(builder), enable_variadic_logical_operators,
@@ -446,7 +452,7 @@ class LegacyConformanceServiceImpl : public ConformanceServiceInterface {
446452
class ModernConformanceServiceImpl : public ConformanceServiceInterface {
447453
public:
448454
static absl::StatusOr<std::unique_ptr<ModernConformanceServiceImpl>> Create(
449-
bool optimize, bool recursive, bool select_optimization,
455+
bool optimize, bool recursive, bool extended_optimizations,
450456
bool enable_variadic_logical_operators, bool enable_pratt_parser) {
451457
google::protobuf::LinkMessageReflection<
452458
cel::expr::conformance::proto3::TestAllTypes>();
@@ -490,7 +496,7 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
490496
}
491497

492498
return absl::WrapUnique(new ModernConformanceServiceImpl(
493-
options, optimize, select_optimization,
499+
options, optimize, extended_optimizations,
494500
enable_variadic_logical_operators, enable_pratt_parser));
495501
}
496502

@@ -512,8 +518,10 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
512518
}
513519
CEL_RETURN_IF_ERROR(cel::EnableReferenceResolver(
514520
builder, cel::ReferenceResolverEnabled::kAlways));
515-
if (enable_select_optimization_) {
521+
if (enable_extended_optimizations_) {
516522
CEL_RETURN_IF_ERROR(cel::extensions::EnableSelectOptimization(builder));
523+
CEL_RETURN_IF_ERROR(
524+
cel::extensions::EnableSetMembershipOptimization(builder));
517525
}
518526

519527
auto& type_registry = builder.type_registry();
@@ -542,6 +550,8 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
542550
builder.function_registry(), options));
543551
CEL_RETURN_IF_ERROR(cel::extensions::RegisterListsFunctions(
544552
builder.function_registry(), options));
553+
CEL_RETURN_IF_ERROR(cel::extensions::RegisterSetsFunctions(
554+
builder.function_registry(), options));
545555

546556
return std::move(builder).Build();
547557
}
@@ -641,12 +651,12 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
641651
private:
642652
ModernConformanceServiceImpl(const RuntimeOptions& options,
643653
bool enable_optimizations,
644-
bool enable_select_optimization,
654+
bool enable_extended_optimizations,
645655
bool enable_variadic_logical_operators,
646656
bool enable_pratt_parser)
647657
: options_(options),
648658
enable_optimizations_(enable_optimizations),
649-
enable_select_optimization_(enable_select_optimization),
659+
enable_extended_optimizations_(enable_extended_optimizations),
650660
enable_variadic_logical_operators_(enable_variadic_logical_operators),
651661
enable_pratt_parser_(enable_pratt_parser) {}
652662

@@ -678,7 +688,7 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
678688

679689
RuntimeOptions options_;
680690
bool enable_optimizations_;
681-
bool enable_select_optimization_;
691+
bool enable_extended_optimizations_;
682692
bool enable_variadic_logical_operators_;
683693
bool enable_pratt_parser_;
684694
};
@@ -693,11 +703,11 @@ absl::StatusOr<std::unique_ptr<ConformanceServiceInterface>>
693703
NewConformanceService(const ConformanceServiceOptions& options) {
694704
if (options.modern) {
695705
return google::api::expr::runtime::ModernConformanceServiceImpl::Create(
696-
options.optimize, options.recursive, options.select_optimization,
706+
options.optimize, options.recursive, options.extended_optimizations,
697707
options.enable_variadic_logical_operators, options.enable_pratt_parser);
698708
} else {
699709
return google::api::expr::runtime::LegacyConformanceServiceImpl::Create(
700-
options.optimize, options.recursive, options.select_optimization,
710+
options.optimize, options.recursive, options.extended_optimizations,
701711
options.enable_variadic_logical_operators, options.enable_pratt_parser);
702712
}
703713
}

‎conformance/service.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ struct ConformanceServiceOptions {
4545
bool modern;
4646
bool arena;
4747
bool recursive;
48-
bool select_optimization;
48+
bool extended_optimizations;
4949
bool enable_variadic_logical_operators = false;
5050
bool enable_pratt_parser = true;
5151
};

‎eval/compiler/flat_expr_builder_extensions.cc‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,8 +303,15 @@ Subexpression* absl_nullable ProgramBuilder::MakeSubexpression(
303303
return it->second.get();
304304
}
305305

306-
bool PlannerContext::IsSubplanInspectable(const cel::Expr& node) const {
307-
return program_builder_.GetSubexpression(&node) != nullptr;
306+
bool PlannerContext::NodeHasInspectablePlan(const cel::Expr& node) const {
307+
const Subexpression* subexpression = program_builder_.GetSubexpression(&node);
308+
if (subexpression == nullptr) {
309+
return false;
310+
}
311+
if (subexpression->IsFlattened()) {
312+
return !subexpression->flattened_elements().empty();
313+
}
314+
return !subexpression->elements().empty();
308315
}
309316

310317
ExecutionPathView PlannerContext::GetSubplan(const cel::Expr& node) {

‎eval/compiler/flat_expr_builder_extensions.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ class PlannerContext {
308308
// Returns true if the subplan is inspectable.
309309
//
310310
// If false, the node is not mapped to a subexpression in the program builder.
311-
bool IsSubplanInspectable(const cel::Expr& node) const;
311+
bool NodeHasInspectablePlan(const cel::Expr& node) const;
312312

313313
// Return a view to the current subplan representing node.
314314
//

‎eval/compiler/flat_expr_builder_extensions_test.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ TEST_F(PlannerContextTest, GetPlan) {
144144
UniquePtrHolds(step_ptrs.a)));
145145

146146
Expr d;
147-
EXPECT_FALSE(context.IsSubplanInspectable(d));
147+
EXPECT_FALSE(context.NodeHasInspectablePlan(d));
148148
EXPECT_THAT(context.GetSubplan(d), IsEmpty());
149149
}
150150

@@ -193,8 +193,8 @@ TEST_F(PlannerContextTest, ExtractPlan) {
193193
type_registry_.GetComposedTypeProvider(),
194194
issue_collector_, program_builder, arena);
195195

196-
EXPECT_TRUE(context.IsSubplanInspectable(a));
197-
EXPECT_TRUE(context.IsSubplanInspectable(b));
196+
EXPECT_TRUE(context.NodeHasInspectablePlan(a));
197+
EXPECT_TRUE(context.NodeHasInspectablePlan(b));
198198

199199
ASSERT_OK_AND_ASSIGN(ExecutionPath extracted, context.ExtractSubplan(b));
200200

@@ -233,7 +233,7 @@ TEST_F(PlannerContextTest, ReplacePlanUpdatesParent) {
233233
type_registry_.GetComposedTypeProvider(),
234234
issue_collector_, program_builder, arena);
235235

236-
EXPECT_TRUE(context.IsSubplanInspectable(a));
236+
EXPECT_TRUE(context.NodeHasInspectablePlan(a));
237237

238238
ASSERT_THAT(context.ReplaceSubplan(c, {}), IsOk());
239239

‎eval/eval/BUILD‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ package_group(
3535
cc_library(
3636
name = "evaluator_core",
3737
srcs = [
38+
"bitset_steps.cc",
3839
"comprehension_step.cc",
3940
"create_list_step.cc",
4041
"equality_steps.cc",
@@ -45,6 +46,7 @@ cc_library(
4546
"logic_step.cc",
4647
],
4748
hdrs = [
49+
"bitset_steps.h",
4850
"comprehension_step.h",
4951
"create_list_step.h",
5052
"equality_steps.h",
@@ -356,6 +358,40 @@ cc_library(
356358
],
357359
)
358360

361+
cc_library(
362+
name = "bitset_steps",
363+
deps = [
364+
":evaluator_core",
365+
],
366+
)
367+
368+
cc_test(
369+
name = "bitset_steps_test",
370+
srcs = [
371+
"bitset_steps_test.cc",
372+
],
373+
deps = [
374+
":attribute_trail",
375+
":evaluator_core",
376+
":expression_step_logic",
377+
"//base:attributes",
378+
"//common:value",
379+
"//common:value_kind",
380+
"//common:value_testing",
381+
"//internal:testing",
382+
"//internal:testing_descriptor_pool",
383+
"//internal:testing_message_factory",
384+
"//runtime:activation",
385+
"//runtime:runtime_options",
386+
"//runtime/internal:runtime_type_provider",
387+
"@com_google_absl//absl/base:nullability",
388+
"@com_google_absl//absl/log:absl_check",
389+
"@com_google_absl//absl/status",
390+
"@com_google_absl//absl/status:status_matchers",
391+
"@com_google_protobuf//:protobuf",
392+
],
393+
)
394+
359395
cc_test(
360396
name = "equality_steps_test",
361397
srcs = [

0 commit comments

Comments
 (0)