From 438bde47817863db3f43ff494db1cc77a40840a4 Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Wed, 13 Dec 2023 15:37:14 -0800 Subject: [PATCH] Add whitebox test for query plan reordering --- flecs.c | 34 ++++++++++++++----- src/addons/rules/api.c | 2 +- src/addons/rules/compile.c | 32 ++++++++++++++---- test/addons/project.json | 4 ++- test/addons/src/RulesBasic.c | 63 ++++++++++++++++++++++++++++++++++++ test/addons/src/main.c | 12 ++++++- 6 files changed, 129 insertions(+), 18 deletions(-) diff --git a/flecs.c b/flecs.c index 42dfca740b..52193d38dc 100644 --- a/flecs.c +++ b/flecs.c @@ -60660,7 +60660,7 @@ char* ecs_rule_str_w_profile( #ifdef FLECS_LOG char *str = ecs_strbuf_get(&buf); - flecs_colorize_buf(str, true, &buf); + flecs_colorize_buf(str, ecs_os_api.flags_ & EcsOsApiLogWithColors, &buf); ecs_os_free(str); #endif return ecs_strbuf_get(&buf); @@ -62462,6 +62462,24 @@ int flecs_rule_compile_term( return -1; } +static +bool flecs_rule_var_is_unknown( + ecs_rule_t *rule, + ecs_var_id_t var_id, + ecs_rule_compile_ctx_t *ctx) +{ + ecs_rule_var_t *vars = rule->vars; + if (ctx->written & (1ull << var_id)) { + return false; + } else { + ecs_var_id_t table_var = vars[var_id].table_id; + if (table_var != EcsVarNone) { + return flecs_rule_var_is_unknown(rule, table_var, ctx); + } + } + return true; +} + static bool flecs_rule_term_is_unknown( ecs_rule_t *rule, @@ -62473,7 +62491,7 @@ bool flecs_rule_term_is_unknown( &dummy.first, EcsRuleFirst, EcsVarEntity, ctx, false); flecs_rule_compile_term_id(NULL, rule, &dummy, &term->second, &dummy.second, EcsRuleSecond, EcsVarEntity, ctx, false); - flecs_rule_compile_term_id(NULL, rule, &dummy, &term->second, + flecs_rule_compile_term_id(NULL, rule, &dummy, &term->src, &dummy.src, EcsRuleSrc, EcsVarAny, ctx, false); bool has_vars = dummy.flags & @@ -62487,17 +62505,17 @@ bool flecs_rule_term_is_unknown( } if (dummy.flags & (EcsRuleIsVar << EcsRuleFirst)) { - if (ctx->written & (1 << dummy.first.var)) { + if (!flecs_rule_var_is_unknown(rule, dummy.first.var, ctx)) { return false; } } if (dummy.flags & (EcsRuleIsVar << EcsRuleSecond)) { - if (ctx->written & (1 << dummy.second.var)) { + if (!flecs_rule_var_is_unknown(rule, dummy.second.var, ctx)) { return false; } } if (dummy.flags & (EcsRuleIsVar << EcsRuleSrc)) { - if (ctx->written & (1 << dummy.src.var)) { + if (!flecs_rule_var_is_unknown(rule, dummy.src.var, ctx)) { return false; } } @@ -62518,7 +62536,7 @@ int32_t flecs_rule_term_next_known( for (i = offset; i < count; i ++) { ecs_term_t *term = &terms[i]; - if (compiled & (1 << i)) { + if (compiled & (1ull << i)) { continue; } @@ -62593,7 +62611,7 @@ int flecs_rule_compile( ecs_term_t *term = &terms[i]; int32_t compile = i; - if (compiled & (1 << i)) { + if (compiled & (1ull << i)) { continue; /* Already compiled */ } @@ -62623,7 +62641,7 @@ int flecs_rule_compile( return -1; } - compiled |= (1 << compile); + compiled |= (1ull << compile); } ecs_var_id_t this_id = flecs_rule_find_var_id(rule, "This", EcsVarEntity); diff --git a/src/addons/rules/api.c b/src/addons/rules/api.c index ee9e3a7ae4..3a0f5c51db 100644 --- a/src/addons/rules/api.c +++ b/src/addons/rules/api.c @@ -295,7 +295,7 @@ char* ecs_rule_str_w_profile( #ifdef FLECS_LOG char *str = ecs_strbuf_get(&buf); - flecs_colorize_buf(str, true, &buf); + flecs_colorize_buf(str, ecs_os_api.flags_ & EcsOsApiLogWithColors, &buf); ecs_os_free(str); #endif return ecs_strbuf_get(&buf); diff --git a/src/addons/rules/compile.c b/src/addons/rules/compile.c index ae81c3fbe2..82df857586 100644 --- a/src/addons/rules/compile.c +++ b/src/addons/rules/compile.c @@ -1684,6 +1684,24 @@ int flecs_rule_compile_term( return -1; } +static +bool flecs_rule_var_is_unknown( + ecs_rule_t *rule, + ecs_var_id_t var_id, + ecs_rule_compile_ctx_t *ctx) +{ + ecs_rule_var_t *vars = rule->vars; + if (ctx->written & (1ull << var_id)) { + return false; + } else { + ecs_var_id_t table_var = vars[var_id].table_id; + if (table_var != EcsVarNone) { + return flecs_rule_var_is_unknown(rule, table_var, ctx); + } + } + return true; +} + static bool flecs_rule_term_is_unknown( ecs_rule_t *rule, @@ -1695,7 +1713,7 @@ bool flecs_rule_term_is_unknown( &dummy.first, EcsRuleFirst, EcsVarEntity, ctx, false); flecs_rule_compile_term_id(NULL, rule, &dummy, &term->second, &dummy.second, EcsRuleSecond, EcsVarEntity, ctx, false); - flecs_rule_compile_term_id(NULL, rule, &dummy, &term->second, + flecs_rule_compile_term_id(NULL, rule, &dummy, &term->src, &dummy.src, EcsRuleSrc, EcsVarAny, ctx, false); bool has_vars = dummy.flags & @@ -1709,17 +1727,17 @@ bool flecs_rule_term_is_unknown( } if (dummy.flags & (EcsRuleIsVar << EcsRuleFirst)) { - if (ctx->written & (1 << dummy.first.var)) { + if (!flecs_rule_var_is_unknown(rule, dummy.first.var, ctx)) { return false; } } if (dummy.flags & (EcsRuleIsVar << EcsRuleSecond)) { - if (ctx->written & (1 << dummy.second.var)) { + if (!flecs_rule_var_is_unknown(rule, dummy.second.var, ctx)) { return false; } } if (dummy.flags & (EcsRuleIsVar << EcsRuleSrc)) { - if (ctx->written & (1 << dummy.src.var)) { + if (!flecs_rule_var_is_unknown(rule, dummy.src.var, ctx)) { return false; } } @@ -1740,7 +1758,7 @@ int32_t flecs_rule_term_next_known( for (i = offset; i < count; i ++) { ecs_term_t *term = &terms[i]; - if (compiled & (1 << i)) { + if (compiled & (1ull << i)) { continue; } @@ -1815,7 +1833,7 @@ int flecs_rule_compile( ecs_term_t *term = &terms[i]; int32_t compile = i; - if (compiled & (1 << i)) { + if (compiled & (1ull << i)) { continue; /* Already compiled */ } @@ -1845,7 +1863,7 @@ int flecs_rule_compile( return -1; } - compiled |= (1 << compile); + compiled |= (1ull << compile); } ecs_var_id_t this_id = flecs_rule_find_var_id(rule, "This", EcsVarEntity); diff --git a/test/addons/project.json b/test/addons/project.json index 1d16542f11..44eafb40c0 100644 --- a/test/addons/project.json +++ b/test/addons/project.json @@ -795,7 +795,9 @@ "unknown_before_known_after_or", "unknown_before_known_after_not", "unknown_before_known_after_optional", - "unknown_before_known_after_scope" + "unknown_before_known_after_scope", + "reordered_plan_1", + "reordered_plan_2" ] }, { "id": "RulesVariables", diff --git a/test/addons/src/RulesBasic.c b/test/addons/src/RulesBasic.c index 34afdf378a..7a717b443c 100644 --- a/test/addons/src/RulesBasic.c +++ b/test/addons/src/RulesBasic.c @@ -6200,3 +6200,66 @@ void RulesBasic_unknown_before_known_after_scope(void) { ecs_fini(world); } + +void RulesBasic_reordered_plan_1(void) { + ecs_world_t *world = ecs_mini(); + + ECS_TAG(world, Foo); + ECS_TAG(world, Bar); + + ecs_rule_t *r = ecs_rule(world, { + .expr = "Foo, ChildOf($this, $p, $gp, $ggp), Bar($ggp)" + }); + + ecs_log_enable_colors(false); + + const char *expect = + HEAD " 0. [-1, 1] setids " + LINE " 1. [ 0, 2] selfup $[this] (Foo)" + LINE " 2. [ 1, 3] and $[this] (ChildOf, $p)" + LINE " 3. [ 2, 4] and $p (ChildOf, $gp)" + LINE " 4. [ 3, 5] and $gp (ChildOf, $ggp)" + LINE " 5. [ 4, 6] selfup $ggp (Bar)" + LINE " 6. [ 5, 7] setvars " + LINE " 7. [ 6, 8] yield " + LINE ""; + char *plan = ecs_rule_str(r); + + test_str(expect, plan); + ecs_os_free(plan); + + ecs_fini(world); +} + +void RulesBasic_reordered_plan_2(void) { + ecs_world_t *world = ecs_mini(); + + ECS_TAG(world, Foo); + ECS_TAG(world, Bar); + + ecs_rule_t *r = ecs_rule(world, { + .expr = "Foo($ggp), ChildOf($this, $p, $gp, $ggp), Bar($this)" + }); + + ecs_log_enable_colors(false); + + const char *expect = + HEAD " 0. [-1, 1] setids " + LINE " 1. [ 0, 2] selfup $[ggp] (Foo)" + LINE " 2. [ 1, 3] each $ggp ($[ggp])" + LINE " 3. [ 2, 4] and $[gp] (ChildOf, $ggp)" + LINE " 4. [ 3, 5] each $gp ($[gp])" + LINE " 5. [ 4, 6] and $[p] (ChildOf, $gp)" + LINE " 6. [ 5, 7] each $p ($[p])" + LINE " 7. [ 6, 8] and $[this] (ChildOf, $p)" + LINE " 8. [ 7, 9] selfup $[this] (Bar)" + LINE " 9. [ 8, 10] setvars " + LINE "10. [ 9, 11] yield " + LINE ""; + char *plan = ecs_rule_str(r); + + test_str(expect, plan); + ecs_os_free(plan); + + ecs_fini(world); +} diff --git a/test/addons/src/main.c b/test/addons/src/main.c index 9379537c3d..4f76d4eb4d 100644 --- a/test/addons/src/main.c +++ b/test/addons/src/main.c @@ -781,6 +781,8 @@ void RulesBasic_unknown_before_known_after_or(void); void RulesBasic_unknown_before_known_after_not(void); void RulesBasic_unknown_before_known_after_optional(void); void RulesBasic_unknown_before_known_after_scope(void); +void RulesBasic_reordered_plan_1(void); +void RulesBasic_reordered_plan_2(void); // Testsuite 'RulesVariables' void RulesVariables_1_ent_src_w_var(void); @@ -4820,6 +4822,14 @@ bake_test_case RulesBasic_testcases[] = { { "unknown_before_known_after_scope", RulesBasic_unknown_before_known_after_scope + }, + { + "reordered_plan_1", + RulesBasic_reordered_plan_1 + }, + { + "reordered_plan_2", + RulesBasic_reordered_plan_2 } }; @@ -8581,7 +8591,7 @@ static bake_test_suite suites[] = { "RulesBasic", NULL, NULL, - 130, + 132, RulesBasic_testcases }, {