Skip to content

Commit

Permalink
Don't auto-cache query with only not/optional terms
Browse files Browse the repository at this point in the history
  • Loading branch information
@SanderMertens
SanderMertens committed Sep 22, 2024
1 parent b2cf189 commit a1a67ae
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 12 deletions.
31 changes: 26 additions & 5 deletions distr/flecs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}
}

Expand Down
31 changes: 26 additions & 5 deletions src/query/api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}
}

Expand Down
5 changes: 4 additions & 1 deletion test/query/project.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 4 additions & 0 deletions test/query/src/Cached.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
103 changes: 103 additions & 0 deletions test/query/src/Plan.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Loading

0 comments on commit a1a67ae

Please sign in to comment.