diff --git a/distr/flecs.c b/distr/flecs.c index 24e9af87b..e493d6b00 100644 --- a/distr/flecs.c +++ b/distr/flecs.c @@ -32005,13 +32005,13 @@ int flecs_query_set_caching_policy( const ecs_query_desc_t *desc) { ecs_query_cache_kind_t kind = desc->cache_kind; + bool group_order_by = desc->group_by || desc->group_by_callback || + desc->order_by || desc->order_by_callback; /* If caching policy is default, try to pick a policy that does the right * thing in most cases. */ if (kind == EcsQueryCacheDefault) { - if (desc->entity || desc->group_by || desc->group_by_callback || - desc->order_by || desc->order_by_callback) - { + if (desc->entity || group_order_by) { /* If the query is created with an entity handle (typically * indicating that the query is named or belongs to a system) the * chance is very high that the query will be reused, so enable @@ -32059,8 +32059,29 @@ int flecs_query_set_caching_policy( /* Same for when the query has no cacheable terms */ impl->pub.cache_kind = EcsQueryCacheNone; } else { - /* Part of the query is cacheable */ - impl->pub.cache_kind = EcsQueryCacheAuto; + /* Part of the query is cacheable. Make sure to only create a cache + * if the cacheable part of the query contains not just not/optional + * terms, as this would build a cache that contains all tables. */ + int32_t not_optional_terms = 0, cacheable_terms = 0; + if (!group_order_by) { + int32_t i, term_count = impl->pub.term_count; + const ecs_term_t *terms = impl->pub.terms; + for (i = 0; i < term_count; i ++) { + const ecs_term_t *term = &terms[i]; + if (term->flags_ & EcsTermIsCacheable) { + cacheable_terms ++; + if (term->oper == EcsNot || term->oper == EcsOptional) { + not_optional_terms ++; + } + } + } + } + + if (group_order_by || cacheable_terms != not_optional_terms) { + impl->pub.cache_kind = EcsQueryCacheAuto; + } else { + impl->pub.cache_kind = EcsQueryCacheNone; + } } } diff --git a/src/query/api.c b/src/query/api.c index 83797a9e0..af843a811 100644 --- a/src/query/api.c +++ b/src/query/api.c @@ -73,13 +73,13 @@ int flecs_query_set_caching_policy( const ecs_query_desc_t *desc) { ecs_query_cache_kind_t kind = desc->cache_kind; + bool group_order_by = desc->group_by || desc->group_by_callback || + desc->order_by || desc->order_by_callback; /* If caching policy is default, try to pick a policy that does the right * thing in most cases. */ if (kind == EcsQueryCacheDefault) { - if (desc->entity || desc->group_by || desc->group_by_callback || - desc->order_by || desc->order_by_callback) - { + if (desc->entity || group_order_by) { /* If the query is created with an entity handle (typically * indicating that the query is named or belongs to a system) the * chance is very high that the query will be reused, so enable @@ -127,8 +127,29 @@ int flecs_query_set_caching_policy( /* Same for when the query has no cacheable terms */ impl->pub.cache_kind = EcsQueryCacheNone; } else { - /* Part of the query is cacheable */ - impl->pub.cache_kind = EcsQueryCacheAuto; + /* Part of the query is cacheable. Make sure to only create a cache + * if the cacheable part of the query contains not just not/optional + * terms, as this would build a cache that contains all tables. */ + int32_t not_optional_terms = 0, cacheable_terms = 0; + if (!group_order_by) { + int32_t i, term_count = impl->pub.term_count; + const ecs_term_t *terms = impl->pub.terms; + for (i = 0; i < term_count; i ++) { + const ecs_term_t *term = &terms[i]; + if (term->flags_ & EcsTermIsCacheable) { + cacheable_terms ++; + if (term->oper == EcsNot || term->oper == EcsOptional) { + not_optional_terms ++; + } + } + } + } + + if (group_order_by || cacheable_terms != not_optional_terms) { + impl->pub.cache_kind = EcsQueryCacheAuto; + } else { + impl->pub.cache_kind = EcsQueryCacheNone; + } } } diff --git a/test/query/project.json b/test/query/project.json index 26ed2dc8a..278b5da17 100644 --- a/test/query/project.json +++ b/test/query/project.json @@ -779,7 +779,10 @@ "cached_isa_tgt", "cached_isa_tgt_w_self_second", "cached_isa_tgt_no_expr", - "cached_isa_tgt_w_self_second_no_expr" + "cached_isa_tgt_w_self_second_no_expr", + "cached_w_not_and_uncacheable", + "cached_w_optional_and_uncacheable", + "cached_w_not_optional_and_uncacheable" ] }, { "id": "Variables", diff --git a/test/query/src/Cached.c b/test/query/src/Cached.c index f2d60c01c..70659bb1d 100644 --- a/test/query/src/Cached.c +++ b/test/query/src/Cached.c @@ -902,6 +902,8 @@ void Cached_singleton_w_optional_new_empty_table(void) { test_assert(total_count == (prev_total_count - 1)); + ecs_query_fini(q); + ecs_fini(world); } @@ -958,6 +960,7 @@ void Cached_singleton_w_optional_new_empty_non_empty_table(void) { test_int(count, 1); test_assert(total_count == prev_total_count); + ecs_query_fini(q); ecs_fini(world); } @@ -1017,6 +1020,7 @@ void Cached_singleton_w_optional_new_unset_tables(void) { test_int(count, 1); test_assert(total_count == prev_total_count); + ecs_query_fini(q); ecs_fini(world); } diff --git a/test/query/src/Plan.c b/test/query/src/Plan.c index 7f561102f..d4a9dcd62 100644 --- a/test/query/src/Plan.c +++ b/test/query/src/Plan.c @@ -2586,3 +2586,106 @@ void Plan_cached_isa_tgt_w_self_second_no_expr(void) { ecs_fini(world); } + +void Plan_cached_w_not_and_uncacheable(void) { + ecs_world_t *world = ecs_mini(); + + ECS_TAG(world, Tgt); + ECS_TAG(world, Foo); + + ecs_query_t *q = ecs_query(world, { + .expr = "(IsA, Tgt), !Foo", + .cache_kind = EcsQueryCacheAuto + }); + + test_assert(q != NULL); + + ecs_log_enable_colors(false); + + const char *expect = + HEAD " 0. [-1, 1] setids " + LINE " 1. [ 0, 2] trav $[this] (IsA, Tgt)" + LINE " 2. [ 1, 4] not " + LINE " 3. [ 2, 4] and $[this] (Foo)" + LINE " 4. [ 2, 5] end $[this] (Foo)" + LINE " 5. [ 4, 6] yield " + LINE ""; + char *plan = ecs_query_plan(q); + + test_str(expect, plan); + ecs_os_free(plan); + + ecs_query_fini(q); + + ecs_fini(world); +} + +void Plan_cached_w_optional_and_uncacheable(void) { + ecs_world_t *world = ecs_mini(); + + ECS_TAG(world, Tgt); + ECS_TAG(world, Foo); + + ecs_query_t *q = ecs_query(world, { + .expr = "(IsA, Tgt), ?Foo", + .cache_kind = EcsQueryCacheAuto + }); + + test_assert(q != NULL); + + ecs_log_enable_colors(false); + + const char *expect = + HEAD " 0. [-1, 1] setids " + LINE " 1. [ 0, 2] trav $[this] (IsA, Tgt)" + LINE " 2. [ 1, 4] option " + LINE " 3. [ 2, 4] and $[this] (Foo)" + LINE " 4. [ 2, 5] end $[this] (Foo)" + LINE " 5. [ 4, 6] yield " + LINE ""; + char *plan = ecs_query_plan(q); + + test_str(expect, plan); + ecs_os_free(plan); + + ecs_query_fini(q); + + ecs_fini(world); +} + +void Plan_cached_w_not_optional_and_uncacheable(void) { + ecs_world_t *world = ecs_mini(); + + ECS_TAG(world, Tgt); + ECS_TAG(world, Foo); + ECS_TAG(world, Bar); + + ecs_query_t *q = ecs_query(world, { + .expr = "(IsA, Tgt), !Foo, ?Bar", + .cache_kind = EcsQueryCacheAuto + }); + + test_assert(q != NULL); + + ecs_log_enable_colors(false); + + const char *expect = + HEAD " 0. [-1, 1] setids " + LINE " 1. [ 0, 2] trav $[this] (IsA, Tgt)" + LINE " 2. [ 1, 4] not " + LINE " 3. [ 2, 4] and $[this] (Foo)" + LINE " 4. [ 2, 5] end $[this] (Foo)" + LINE " 5. [ 4, 7] option " + LINE " 6. [ 5, 7] and $[this] (Bar)" + LINE " 7. [ 5, 8] end $[this] (Bar)" + LINE " 8. [ 7, 9] yield " + LINE ""; + char *plan = ecs_query_plan(q); + + test_str(expect, plan); + ecs_os_free(plan); + + ecs_query_fini(q); + + ecs_fini(world); +} diff --git a/test/query/src/Traversal.c b/test/query/src/Traversal.c index 43f99d8d8..b33739007 100644 --- a/test/query/src/Traversal.c +++ b/test/query/src/Traversal.c @@ -9946,6 +9946,8 @@ void Traversal_this_up_childof_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -9979,6 +9981,8 @@ void Traversal_this_up_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10015,6 +10019,8 @@ void Traversal_this_up_isa_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10051,6 +10057,8 @@ void Traversal_this_up_isa_childof_isa(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10090,6 +10098,8 @@ void Traversal_this_up_isa_childof_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10131,6 +10141,8 @@ void Traversal_this_self_up_childof_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10169,6 +10181,8 @@ void Traversal_this_self_up_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10210,6 +10224,8 @@ void Traversal_this_self_up_isa_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10251,6 +10267,8 @@ void Traversal_this_self_up_isa_childof_isa(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10295,6 +10313,8 @@ void Traversal_this_self_up_isa_childof_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10329,6 +10349,8 @@ void Traversal_this_written_up_childof_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10365,6 +10387,8 @@ void Traversal_this_written_up_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10404,6 +10428,8 @@ void Traversal_this_written_up_isa_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10438,6 +10464,8 @@ void Traversal_this_written_up_isa_childof_isa(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10475,6 +10503,8 @@ void Traversal_this_written_up_isa_childof_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10509,6 +10539,8 @@ void Traversal_this_written_self_up_childof_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10545,6 +10577,8 @@ void Traversal_this_written_self_up_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10583,6 +10617,8 @@ void Traversal_this_written_self_up_isa_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10617,6 +10653,8 @@ void Traversal_this_written_self_up_isa_childof_isa(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } @@ -10654,5 +10692,7 @@ void Traversal_this_written_self_up_isa_childof_isa_childof(void) { test_bool(false, ecs_query_next(&it)); } + ecs_query_fini(q); + ecs_fini(world); } diff --git a/test/query/src/main.c b/test/query/src/main.c index d0cc0d9b2..79c7d9b85 100644 --- a/test/query/src/main.c +++ b/test/query/src/main.c @@ -760,6 +760,9 @@ void Plan_cached_isa_tgt(void); void Plan_cached_isa_tgt_w_self_second(void); void Plan_cached_isa_tgt_no_expr(void); void Plan_cached_isa_tgt_w_self_second_no_expr(void); +void Plan_cached_w_not_and_uncacheable(void); +void Plan_cached_w_optional_and_uncacheable(void); +void Plan_cached_w_not_optional_and_uncacheable(void); // Testsuite 'Variables' void Variables_setup(void); @@ -5070,6 +5073,18 @@ bake_test_case Plan_testcases[] = { { "cached_isa_tgt_w_self_second_no_expr", Plan_cached_isa_tgt_w_self_second_no_expr + }, + { + "cached_w_not_and_uncacheable", + Plan_cached_w_not_and_uncacheable + }, + { + "cached_w_optional_and_uncacheable", + Plan_cached_w_optional_and_uncacheable + }, + { + "cached_w_not_optional_and_uncacheable", + Plan_cached_w_not_optional_and_uncacheable } }; @@ -10360,7 +10375,7 @@ static bake_test_suite suites[] = { "Plan", NULL, NULL, - 76, + 79, Plan_testcases }, {