Allow dense iteration for FilteredEntity(Ref|Mut) in more cases.

[chescock]

Objective

Improve the performance of some dynamic queries with FilteredEntity(Ref|Mut) by allowing dense iteration in more cases, and remove a call to the sort-of deprecated Access::component_reads_and_writes() method.

QueryBuilder currently requires sparse iteration if any sparse set components may be read. We do need sparse iteration if sparse set components are used in the filters, but FilteredEntityRef can still perform dense iteration when reading optional components or when reading all components.

Note that the optional case is different from Option, which performs sparse iteration when the inner query is sparse so that it can cache whether the inner query matches for an entire archetype.

Solution

Change FilteredEntity(Ref|Mut) to have IS_DENSE = true. It used to require sparse iteration in order to filter the Access for each archetype, but #15207 changed it to copy the entire access.

Change QueryBuilder::is_dense() to check D::IS_DENSE && F::IS_DENSE instead of looking at the component reads and writes.
QueryBuilder::is_dense() still checks the filters, so builder.data::<&Sparse>() will still cause sparse iteration, but builder.data::<Option<&Sparse>>() no longer will.

I believe this is sound, even in the presence of query transmutes. The only WorldQuery implementations that rely on a sparse query being sparse for soundness are &, &mut, Ref, and Mut, but they can only be transmuted to if the component is in the required set. If a dynamic query has the component in the required set, then it appears in the filters and the query will use sparse iteration.

Note that Option and Has will misbehave and report None and false for all entities if they do a dense query while wrapping a sparse component, but they won't cause UB. And it's already possible to hit that case by transmuting from Query<EntityMut> to Query<Option<&Sparse>>.

[alice-i-cecile]

Note that Option and Has will misbehave and report None and false for all entities if they do a dense query while wrapping a sparse component, but they won't cause UB. And it's already possible to hit that case by transmuting from Query to Query<Option<&Sparse>>.

Can you open an issue for this? :)

[Victoronz]

And it's already possible to hit that case by transmuting from Query<EntityMut> to Query<Option<&Sparse>>.

This is possible? This is not documented under Query::transmute_lens, and I've never heard of this kind of transmute before, do you know what PR introduced Query<EntityMut> -> Query<Option<T>> transmutes?

[chescock]

This is possible? This is not documented under Query::transmute_lens, and I've never heard of this kind of transmute before, do you know what PR introduced Query<EntityMut> -> Query<Option<T>> transmutes?

It does work! EntityMut has write access to all components, but doesn't require any of them. So it can't transmute to &T, which requires T, but it can transmute to Option<&T>, which reads it but does not require it.

I confess I only looked at the implementation and not the documentation, so I didn't realize it wasn't documented :). It looks like the subset check hasn't changed much since transmutes were introduced, so I think it's always worked.

Looking at the documentation now, that list doesn't seem especially exhaustive. You can also add Has<T> for any T, or go from AnyOf<(T, U)> to Option<T>. And there can always be third-party WorldQuery implementations.