Reduce code duplication by using QueryIterationCursor in QueryIter (#4733)

# Objective
We have duplicated code between `QueryIter` and `QueryIterationCursor`. Reuse that code.

## Solution
 - Reuse `QueryIterationCursor` inside `QueryIter`.
 - Slim down `QueryIter` by removing the `&'w World`. It was only being used by the `size_hint` and `ExactSizeIterator` impls, which can use the QueryState and &Archetypes in the type already.
 - Benchmark to make sure there is no significant regression.

Relevant benchmark results seem to show that there is no tangible difference between the two. Everything seems to be either identical or within a workable margin of error here.

```
group                                          embed-cursor                            main
-----                                          ------------                            ----
fragmented_iter/base                           1.00   387.4±19.70ns        ? ?/sec     1.07   413.1±27.95ns        ? ?/sec
many_maps_iter                                 1.00     27.3±0.22ms        ? ?/sec     1.00     27.4±0.10ms        ? ?/sec
simple_iter/base                               1.00     13.8±0.07µs        ? ?/sec     1.00     13.7±0.17µs        ? ?/sec
simple_iter/sparse                             1.00     61.9±0.37µs        ? ?/sec     1.00     62.2±0.64µs        ? ?/sec
simple_iter/system                             1.00     13.7±0.34µs        ? ?/sec     1.00     13.7±0.10µs        ? ?/sec
sparse_fragmented_iter/base                    1.00     11.0±0.54ns        ? ?/sec     1.03     11.3±0.48ns        ? ?/sec
world_query_iter/50000_entities_sparse         1.08    105.0±2.68µs        ? ?/sec     1.00     97.5±2.18µs        ? ?/sec
world_query_iter/50000_entities_table          1.00     27.3±0.13µs        ? ?/sec     1.00     27.3±0.37µs        ? ?/sec
```
This commit is contained in:
James Liu 2022-05-18 18:34:52 +00:00
parent 15acd6f45d
commit 2c93b5cf73

View file

@ -16,13 +16,7 @@ pub struct QueryIter<'w, 's, Q: WorldQuery, QF: Fetch<'w, State = Q::State>, F:
tables: &'w Tables,
archetypes: &'w Archetypes,
query_state: &'s QueryState<Q, F>,
world: &'w World,
table_id_iter: std::slice::Iter<'s, TableId>,
archetype_id_iter: std::slice::Iter<'s, ArchetypeId>,
fetch: QF,
filter: QueryFetch<'w, F>,
current_len: usize,
current_index: usize,
cursor: QueryIterationCursor<'w, 's, Q, QF, F>,
}
impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> QueryIter<'w, 's, Q, QF, F>
@ -40,30 +34,11 @@ where
last_change_tick: u32,
change_tick: u32,
) -> Self {
let fetch = QF::init(
world,
&query_state.fetch_state,
last_change_tick,
change_tick,
);
let filter = QueryFetch::<F>::init(
world,
&query_state.filter_state,
last_change_tick,
change_tick,
);
QueryIter {
world,
query_state,
tables: &world.storages().tables,
archetypes: &world.archetypes,
fetch,
filter,
table_id_iter: query_state.matched_table_ids.iter(),
archetype_id_iter: query_state.matched_archetype_ids.iter(),
current_len: 0,
current_index: 0,
cursor: QueryIterationCursor::init(world, query_state, last_change_tick, change_tick),
}
}
}
@ -74,73 +49,20 @@ where
{
type Item = QF::Item;
// NOTE: If you are changing query iteration code, remember to update the following places, where relevant:
// QueryIter, QueryIterationCursor, QueryState::for_each_unchecked_manual, QueryState::par_for_each_unchecked_manual
// We can't currently reuse QueryIterationCursor in QueryIter for performance reasons. See #1763 for context.
#[inline(always)]
fn next(&mut self) -> Option<Self::Item> {
unsafe {
if QF::IS_DENSE && <QueryFetch<'static, F>>::IS_DENSE {
loop {
if self.current_index == self.current_len {
let table_id = self.table_id_iter.next()?;
let table = &self.tables[*table_id];
self.fetch.set_table(&self.query_state.fetch_state, table);
self.filter.set_table(&self.query_state.filter_state, table);
self.current_len = table.len();
self.current_index = 0;
continue;
}
if !self.filter.table_filter_fetch(self.current_index) {
self.current_index += 1;
continue;
}
let item = self.fetch.table_fetch(self.current_index);
self.current_index += 1;
return Some(item);
}
} else {
loop {
if self.current_index == self.current_len {
let archetype_id = self.archetype_id_iter.next()?;
let archetype = &self.archetypes[*archetype_id];
self.fetch.set_archetype(
&self.query_state.fetch_state,
archetype,
self.tables,
);
self.filter.set_archetype(
&self.query_state.filter_state,
archetype,
self.tables,
);
self.current_len = archetype.len();
self.current_index = 0;
continue;
}
if !self.filter.archetype_filter_fetch(self.current_index) {
self.current_index += 1;
continue;
}
let item = self.fetch.archetype_fetch(self.current_index);
self.current_index += 1;
return Some(item);
}
}
self.cursor
.next(self.tables, self.archetypes, self.query_state)
}
}
fn size_hint(&self) -> (usize, Option<usize>) {
let max_size = self
.query_state
.matched_archetypes
.ones()
.map(|index| self.world.archetypes[ArchetypeId::new(index)].len())
.matched_archetype_ids
.iter()
.map(|id| self.archetypes[*id].len())
.sum();
let archetype_query = F::Fetch::IS_ARCHETYPAL && QF::IS_ARCHETYPAL;
@ -153,7 +75,6 @@ pub struct QueryCombinationIter<'w, 's, Q: WorldQuery, F: WorldQuery, const K: u
tables: &'w Tables,
archetypes: &'w Archetypes,
query_state: &'s QueryState<Q, F>,
world: &'w World,
cursors: [QueryIterationCursor<'w, 's, Q, QueryFetch<'w, Q>, F>; K],
}
@ -195,7 +116,6 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> QueryCombinationIter<
}
QueryCombinationIter {
world,
query_state,
tables: &world.storages().tables,
archetypes: &world.archetypes,
@ -289,9 +209,9 @@ where
let max_size: usize = self
.query_state
.matched_archetypes
.ones()
.map(|index| self.world.archetypes[ArchetypeId::new(index)].len())
.matched_archetype_ids
.iter()
.map(|id| self.archetypes[*id].len())
.sum();
if max_size < K {
@ -324,9 +244,9 @@ where
{
fn len(&self) -> usize {
self.query_state
.matched_archetypes
.ones()
.map(|index| self.world.archetypes[ArchetypeId::new(index)].len())
.matched_archetype_ids
.iter()
.map(|id| self.archetypes[*id].len())
.sum()
}
}
@ -363,6 +283,8 @@ impl<'w, 's, Q: WorldQuery, QF, F: WorldQuery> QueryIterationCursor<'w, 's, Q, Q
where
QF: Fetch<'w, State = Q::State>,
{
const IS_DENSE: bool = QF::IS_DENSE && <QueryFetch<'static, F>>::IS_DENSE;
unsafe fn init_empty(
world: &'w World,
query_state: &'s QueryState<Q, F>,
@ -409,7 +331,7 @@ where
#[inline]
unsafe fn peek_last(&mut self) -> Option<QF::Item> {
if self.current_index > 0 {
if QF::IS_DENSE && <QueryFetch<'static, F>>::IS_DENSE {
if Self::IS_DENSE {
Some(self.fetch.table_fetch(self.current_index - 1))
} else {
Some(self.fetch.archetype_fetch(self.current_index - 1))
@ -420,8 +342,7 @@ where
}
// NOTE: If you are changing query iteration code, remember to update the following places, where relevant:
// QueryIter, QueryIterationCursor, QueryState::for_each_unchecked_manual, QueryState::par_for_each_unchecked_manual
// We can't currently reuse QueryIterationCursor in QueryIter for performance reasons. See #1763 for context.
// QueryIterationCursor, QueryState::for_each_unchecked_manual, QueryState::par_for_each_unchecked_manual
#[inline(always)]
unsafe fn next(
&mut self,
@ -429,7 +350,7 @@ where
archetypes: &'w Archetypes,
query_state: &'s QueryState<Q, F>,
) -> Option<QF::Item> {
if QF::IS_DENSE && <QueryFetch<'static, F>>::IS_DENSE {
if Self::IS_DENSE {
loop {
if self.current_index == self.current_len {
let table_id = self.table_id_iter.next()?;