Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Query bugfixes/improvements #1368

Merged
merged 3 commits into from
Sep 22, 2024
Merged

Query bugfixes/improvements #1368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
82e262f
Fix issue with matching prefab/disabled entities for queries that sta…
SanderMertens Sep 21, 2024
ff74189
Allow (IsA, tgt|self) queries to be cached
SanderMertens Sep 21, 2024
adbc71e
Don't auto-cache query with only not/optional terms
SanderMertens Sep 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 76 additions & 42 deletions distr/flecs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4122,6 +4122,7 @@ void flecs_bootstrap(
ecs_add_id(world, EcsThis, EcsNotQueryable);
ecs_add_id(world, EcsWildcard, EcsNotQueryable);
ecs_add_id(world, EcsAny, EcsNotQueryable);
ecs_add_id(world, EcsVariable, EcsNotQueryable);

/* Tag relationships (relationships that should never have data) */
ecs_add_id(world, EcsIsA, EcsPairIsTag);
Expand Down Expand Up @@ -32004,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 @@ -32058,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 Expand Up @@ -33994,29 +34016,29 @@ int flecs_term_finalize(
ecs_record_t *first_record = flecs_entities_get(world, first_entity);
ecs_table_t *first_table = first_record ? first_record->table : NULL;
if (first_table) {
/* Add traversal flags for transitive relationships */
if (ecs_term_ref_is_set(second) && !((second_flags & EcsTraverseFlags) == EcsSelf)) {
if (!((src->id & EcsIsVariable) && (src_id == EcsAny))) {
if (!((second->id & EcsIsVariable) && (second_id == EcsAny))) {
if (ecs_table_has_id(world, first_table, EcsTransitive)) {
term->flags_ |= EcsTermTransitive;
if (ecs_term_ref_is_set(second)) {
/* Add traversal flags for transitive relationships */
if (!((second_flags & EcsTraverseFlags) == EcsSelf)) {
if (!((src->id & EcsIsVariable) && (src_id == EcsAny))) {
if (!((second->id & EcsIsVariable) && (second_id == EcsAny))) {
if (ecs_table_has_id(world, first_table, EcsTransitive)) {
term->flags_ |= EcsTermTransitive;
}

if (ecs_table_has_id(world, first_table, EcsReflexive)) {
term->flags_ |= EcsTermReflexive;
}
}
}
}
}

if (ECS_IS_PAIR(term->id)) {
if (ecs_table_has_id(world, first_table, EcsReflexive)) {
term->flags_ |= EcsTermReflexive;
}
}

/* Check if term is union */
if (ecs_table_has_id(world, first_table, EcsUnion)) {
/* Any wildcards don't need special handling as they just return
* (Rel, *). */
if (ECS_IS_PAIR(term->id) && ECS_PAIR_SECOND(term->id) != EcsAny) {
term->flags_ |= EcsTermIsUnion;
/* Check if term is union */
if (ecs_table_has_id(world, first_table, EcsUnion)) {
/* Any wildcards don't need special handling as they just return
* (Rel, *). */
if (ECS_IS_PAIR(term->id) && ECS_PAIR_SECOND(term->id) != EcsAny) {
term->flags_ |= EcsTermIsUnion;
}
}
}
}
Expand Down Expand Up @@ -34088,7 +34110,7 @@ int flecs_term_finalize(

if (term->flags_ & EcsTermTransitive) {
trivial_term = false;
cacheable_term = false;
cacheable_term = false;
}

if (term->flags_ & EcsTermIdInherited) {
Expand Down Expand Up @@ -34809,11 +34831,13 @@ bool flecs_query_finalize_simple(
if (ECS_IS_PAIR(id)) {
if (ecs_has_id(world, first, EcsTransitive)) {
term->flags_ |= EcsTermTransitive;
cacheable = false; trivial = false;
trivial = false;
cacheable = false;
}
if (ecs_has_id(world, first, EcsReflexive)) {
term->flags_ |= EcsTermReflexive;
cacheable = false; trivial = false;
trivial = false;
cacheable = false;
}
}

Expand Down Expand Up @@ -65631,8 +65655,30 @@ int flecs_query_compile_0_src(
return -1;
}

static
ecs_flags32_t flecs_query_to_table_flags(
const ecs_query_t *q)
{
ecs_flags32_t query_flags = q->flags;
if (!(query_flags & EcsQueryMatchDisabled) ||
!(query_flags & EcsQueryMatchPrefab))
{
ecs_flags32_t table_flags = EcsTableNotQueryable;
if (!(query_flags & EcsQueryMatchDisabled)) {
table_flags |= EcsTableIsDisabled;
}
if (!(query_flags & EcsQueryMatchPrefab)) {
table_flags |= EcsTableIsPrefab;
}

return table_flags;
}
return 0;
}

static
bool flecs_query_select_all(
const ecs_query_t *q,
ecs_term_t *term,
ecs_query_op_t *op,
ecs_var_id_t src_var,
Expand Down Expand Up @@ -65660,6 +65706,7 @@ bool flecs_query_select_all(
match_any.flags |= (EcsQueryIsEntity << EcsQuerySecond);
}
match_any.written = (1ull << src_var);
match_any.other = flecs_itolbl(flecs_query_to_table_flags(q));
flecs_query_op_insert(&match_any, ctx);
flecs_query_write_ctx(op->src.var, ctx, false);

Expand Down Expand Up @@ -66073,7 +66120,7 @@ int flecs_query_compile_term(
* written to yet, insert instruction that selects all entities so we have
* something to match the optional/not against. */
if (src_is_var && !src_written && !src_is_wildcard && !src_is_lookup) {
src_written = flecs_query_select_all(term, &op, op.src.var, ctx);
src_written = flecs_query_select_all(q, term, &op, op.src.var, ctx);
}

/* A bit of special logic for OR expressions and equality predicates. If the
Expand Down Expand Up @@ -66212,20 +66259,7 @@ int flecs_query_compile_term(
* filtering out disabled/prefab entities is the default and this check is
* cheap to perform on table flags, it's worth special casing. */
if (!src_written && op.src.var == 0) {
ecs_flags32_t query_flags = q->flags;
if (!(query_flags & EcsQueryMatchDisabled) ||
!(query_flags & EcsQueryMatchPrefab))
{
ecs_flags32_t table_flags = EcsTableNotQueryable;
if (!(query_flags & EcsQueryMatchDisabled)) {
table_flags |= EcsTableIsDisabled;
}
if (!(query_flags & EcsQueryMatchPrefab)) {
table_flags |= EcsTableIsPrefab;
}

op.other = flecs_itolbl(table_flags);
}
op.other = flecs_itolbl(flecs_query_to_table_flags(q));
}

/* After evaluating a term, a used variable is always written */
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,7 @@ void flecs_bootstrap(
ecs_add_id(world, EcsThis, EcsNotQueryable);
ecs_add_id(world, EcsWildcard, EcsNotQueryable);
ecs_add_id(world, EcsAny, EcsNotQueryable);
ecs_add_id(world, EcsVariable, EcsNotQueryable);

/* Tag relationships (relationships that should never have data) */
ecs_add_id(world, EcsIsA, EcsPairIsTag);
Expand Down
31 changes: 26 additions & 5 deletions src/query/api.c
View file
Open in desktop
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
40 changes: 25 additions & 15 deletions src/query/compiler/compiler_term.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,8 +798,30 @@ int flecs_query_compile_0_src(
return -1;
}

static
ecs_flags32_t flecs_query_to_table_flags(
const ecs_query_t *q)
{
ecs_flags32_t query_flags = q->flags;
if (!(query_flags & EcsQueryMatchDisabled) ||
!(query_flags & EcsQueryMatchPrefab))
{
ecs_flags32_t table_flags = EcsTableNotQueryable;
if (!(query_flags & EcsQueryMatchDisabled)) {
table_flags |= EcsTableIsDisabled;
}
if (!(query_flags & EcsQueryMatchPrefab)) {
table_flags |= EcsTableIsPrefab;
}

return table_flags;
}
return 0;
}

static
bool flecs_query_select_all(
const ecs_query_t *q,
ecs_term_t *term,
ecs_query_op_t *op,
ecs_var_id_t src_var,
Expand Down Expand Up @@ -827,6 +849,7 @@ bool flecs_query_select_all(
match_any.flags |= (EcsQueryIsEntity << EcsQuerySecond);
}
match_any.written = (1ull << src_var);
match_any.other = flecs_itolbl(flecs_query_to_table_flags(q));
flecs_query_op_insert(&match_any, ctx);
flecs_query_write_ctx(op->src.var, ctx, false);

Expand Down Expand Up @@ -1240,7 +1263,7 @@ int flecs_query_compile_term(
* written to yet, insert instruction that selects all entities so we have
* something to match the optional/not against. */
if (src_is_var && !src_written && !src_is_wildcard && !src_is_lookup) {
src_written = flecs_query_select_all(term, &op, op.src.var, ctx);
src_written = flecs_query_select_all(q, term, &op, op.src.var, ctx);
}

/* A bit of special logic for OR expressions and equality predicates. If the
Expand Down Expand Up @@ -1379,20 +1402,7 @@ int flecs_query_compile_term(
* filtering out disabled/prefab entities is the default and this check is
* cheap to perform on table flags, it's worth special casing. */
if (!src_written && op.src.var == 0) {
ecs_flags32_t query_flags = q->flags;
if (!(query_flags & EcsQueryMatchDisabled) ||
!(query_flags & EcsQueryMatchPrefab))
{
ecs_flags32_t table_flags = EcsTableNotQueryable;
if (!(query_flags & EcsQueryMatchDisabled)) {
table_flags |= EcsTableIsDisabled;
}
if (!(query_flags & EcsQueryMatchPrefab)) {
table_flags |= EcsTableIsPrefab;
}

op.other = flecs_itolbl(table_flags);
}
op.other = flecs_itolbl(flecs_query_to_table_flags(q));
}

/* After evaluating a term, a used variable is always written */
Expand Down
Loading