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.

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

Reduced TableRow as Casting #10811

Merged
merged 7 commits into from
Dec 5, 2023
14 changes: 7 additions & 7 deletions crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ unsafe impl<T: Component> WorldQuery for &T {
StorageType::Table => fetch
.table_components
.debug_checked_unwrap()
.get(table_row.index())
.get(table_row.as_usize())
.deref(),
StorageType::SparseSet => fetch
.sparse_set
Expand Down Expand Up @@ -760,10 +760,10 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> {
let (table_components, added_ticks, changed_ticks) =
fetch.table_data.debug_checked_unwrap();
Ref {
value: table_components.get(table_row.index()).deref(),
value: table_components.get(table_row.as_usize()).deref(),
ticks: Ticks {
added: added_ticks.get(table_row.index()).deref(),
changed: changed_ticks.get(table_row.index()).deref(),
added: added_ticks.get(table_row.as_usize()).deref(),
changed: changed_ticks.get(table_row.as_usize()).deref(),
this_run: fetch.this_run,
last_run: fetch.last_run,
},
Expand Down Expand Up @@ -927,10 +927,10 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
let (table_components, added_ticks, changed_ticks) =
fetch.table_data.debug_checked_unwrap();
Mut {
value: table_components.get(table_row.index()).deref_mut(),
value: table_components.get(table_row.as_usize()).deref_mut(),
ticks: TicksMut {
added: added_ticks.get(table_row.index()).deref_mut(),
changed: changed_ticks.get(table_row.index()).deref_mut(),
added: added_ticks.get(table_row.as_usize()).deref_mut(),
changed: changed_ticks.get(table_row.as_usize()).deref_mut(),
this_run: fetch.this_run,
last_run: fetch.last_run,
},
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/query/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ unsafe impl<T: Component> WorldQuery for Added<T> {
StorageType::Table => fetch
.table_ticks
.debug_checked_unwrap()
.get(table_row.index())
.get(table_row.as_usize())
.deref()
.is_newer_than(fetch.last_run, fetch.this_run),
StorageType::SparseSet => {
Expand Down Expand Up @@ -802,7 +802,7 @@ unsafe impl<T: Component> WorldQuery for Changed<T> {
StorageType::Table => fetch
.table_ticks
.debug_checked_unwrap()
.get(table_row.index())
.get(table_row.as_usize())
.deref()
.is_newer_than(fetch.last_run, fetch.this_run),
StorageType::SparseSet => {
Expand Down
15 changes: 12 additions & 3 deletions crates/bevy_ecs/src/query/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ impl<'w, 's, Q: WorldQueryData, F: WorldQueryFilter> QueryIter<'w, 's, Q, F> {
where
Func: FnMut(B, Q::Item<'w>) -> B,
{
assert!(
rows.end <= u32::MAX as usize,
"TableRow is only valid up to u32::MAX"
);

Q::set_table(&mut self.cursor.fetch, &self.query_state.fetch_state, table);
F::set_table(
&mut self.cursor.filter,
Expand All @@ -117,7 +122,7 @@ impl<'w, 's, Q: WorldQueryData, F: WorldQueryFilter> QueryIter<'w, 's, Q, F> {
for row in rows {
// SAFETY: Caller assures `row` in range of the current archetype.
let entity = entities.get_unchecked(row);
let row = TableRow::new(row);
let row = TableRow::from_usize(row);
// SAFETY: set_table was called prior.
// Caller assures `row` in range of the current archetype.
if !F::filter_fetch(&mut self.cursor.filter, *entity, row) {
Expand Down Expand Up @@ -707,7 +712,11 @@ impl<'w, 's, Q: WorldQueryData, F: WorldQueryFilter> QueryIterationCursor<'w, 's
let index = self.current_row - 1;
if Self::IS_DENSE {
let entity = self.table_entities.get_unchecked(index);
Some(Q::fetch(&mut self.fetch, *entity, TableRow::new(index)))
Some(Q::fetch(
&mut self.fetch,
*entity,
TableRow::from_usize(index),
))
} else {
let archetype_entity = self.archetype_entities.get_unchecked(index);
Some(Q::fetch(
Expand Down Expand Up @@ -768,7 +777,7 @@ impl<'w, 's, Q: WorldQueryData, F: WorldQueryFilter> QueryIterationCursor<'w, 's
// SAFETY: set_table was called prior.
// `current_row` is a table row in range of the current table, because if it was not, then the if above would have been executed.
let entity = self.table_entities.get_unchecked(self.current_row);
let row = TableRow::new(self.current_row);
let row = TableRow::from_usize(self.current_row);
if !F::filter_fetch(&mut self.filter, *entity, row) {
self.current_row += 1;
continue;
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ impl<Q: WorldQueryData, F: WorldQueryFilter> QueryState<Q, F> {
self.matched_archetypes.set(archetype_index, true);
self.matched_archetype_ids.push(archetype.id());
}
let table_index = archetype.table_id().index();
let table_index = archetype.table_id().as_usize();
if !self.matched_tables.contains(table_index) {
self.matched_tables.grow(table_index + 1);
self.matched_tables.set(table_index, true);
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/storage/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl<const SEND: bool> Drop for ResourceData<SEND> {

impl<const SEND: bool> ResourceData<SEND> {
/// The only row in the underlying column.
const ROW: TableRow = TableRow::new(0);
const ROW: TableRow = TableRow::from_u32(0);

/// Validates the access to `!Send` resources is only done on the thread they were created from.
///
Expand Down
82 changes: 34 additions & 48 deletions crates/bevy_ecs/src/storage/sparse_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ pub struct ComponentSparseSet {
entities: Vec<EntityIndex>,
#[cfg(debug_assertions)]
entities: Vec<Entity>,
sparse: SparseArray<EntityIndex, u32>,
sparse: SparseArray<EntityIndex, TableRow>,
}

impl ComponentSparseSet {
Expand Down Expand Up @@ -171,13 +171,13 @@ impl ComponentSparseSet {
) {
if let Some(&dense_index) = self.sparse.get(entity.index()) {
#[cfg(debug_assertions)]
assert_eq!(entity, self.entities[dense_index as usize]);
self.dense
.replace(TableRow::new(dense_index as usize), value, change_tick);
assert_eq!(entity, self.entities[dense_index.as_usize()]);
self.dense.replace(dense_index, value, change_tick);
} else {
let dense_index = self.dense.len();
self.dense.push(value, ComponentTicks::new(change_tick));
self.sparse.insert(entity.index(), dense_index as u32);
self.sparse
.insert(entity.index(), TableRow::from_usize(dense_index));
#[cfg(debug_assertions)]
assert_eq!(self.entities.len(), dense_index);
#[cfg(not(debug_assertions))]
Expand All @@ -194,7 +194,7 @@ impl ComponentSparseSet {
{
if let Some(&dense_index) = self.sparse.get(entity.index()) {
#[cfg(debug_assertions)]
assert_eq!(entity, self.entities[dense_index as usize]);
assert_eq!(entity, self.entities[dense_index.as_usize()]);
true
} else {
false
Expand All @@ -209,12 +209,11 @@ impl ComponentSparseSet {
/// Returns `None` if `entity` does not have a component in the sparse set.
#[inline]
pub fn get(&self, entity: Entity) -> Option<Ptr<'_>> {
self.sparse.get(entity.index()).map(|dense_index| {
let dense_index = (*dense_index) as usize;
self.sparse.get(entity.index()).map(|&dense_index| {
#[cfg(debug_assertions)]
assert_eq!(entity, self.entities[dense_index]);
assert_eq!(entity, self.entities[dense_index.as_usize()]);
// SAFETY: if the sparse index points to something in the dense vec, it exists
unsafe { self.dense.get_data_unchecked(TableRow::new(dense_index)) }
unsafe { self.dense.get_data_unchecked(dense_index) }
})
}

Expand All @@ -223,9 +222,9 @@ impl ComponentSparseSet {
/// Returns `None` if `entity` does not have a component in the sparse set.
#[inline]
pub fn get_with_ticks(&self, entity: Entity) -> Option<(Ptr<'_>, TickCells<'_>)> {
let dense_index = TableRow::new(*self.sparse.get(entity.index())? as usize);
let dense_index = *self.sparse.get(entity.index())?;
#[cfg(debug_assertions)]
assert_eq!(entity, self.entities[dense_index.index()]);
assert_eq!(entity, self.entities[dense_index.as_usize()]);
// SAFETY: if the sparse index points to something in the dense vec, it exists
unsafe {
Some((
Expand All @@ -243,69 +242,55 @@ impl ComponentSparseSet {
/// Returns `None` if `entity` does not have a component in the sparse set.
#[inline]
pub fn get_added_tick(&self, entity: Entity) -> Option<&UnsafeCell<Tick>> {
let dense_index = *self.sparse.get(entity.index())? as usize;
let dense_index = *self.sparse.get(entity.index())?;
#[cfg(debug_assertions)]
assert_eq!(entity, self.entities[dense_index]);
assert_eq!(entity, self.entities[dense_index.as_usize()]);
// SAFETY: if the sparse index points to something in the dense vec, it exists
unsafe {
Some(
self.dense
.get_added_tick_unchecked(TableRow::new(dense_index)),
)
}
unsafe { Some(self.dense.get_added_tick_unchecked(dense_index)) }
}

/// Returns a reference to the "changed" tick of the entity's component value.
///
/// Returns `None` if `entity` does not have a component in the sparse set.
#[inline]
pub fn get_changed_tick(&self, entity: Entity) -> Option<&UnsafeCell<Tick>> {
let dense_index = *self.sparse.get(entity.index())? as usize;
let dense_index = *self.sparse.get(entity.index())?;
#[cfg(debug_assertions)]
assert_eq!(entity, self.entities[dense_index]);
assert_eq!(entity, self.entities[dense_index.as_usize()]);
// SAFETY: if the sparse index points to something in the dense vec, it exists
unsafe {
Some(
self.dense
.get_changed_tick_unchecked(TableRow::new(dense_index)),
)
}
unsafe { Some(self.dense.get_changed_tick_unchecked(dense_index)) }
}

/// Returns a reference to the "added" and "changed" ticks of the entity's component value.
///
/// Returns `None` if `entity` does not have a component in the sparse set.
#[inline]
pub fn get_ticks(&self, entity: Entity) -> Option<ComponentTicks> {
let dense_index = *self.sparse.get(entity.index())? as usize;
let dense_index = *self.sparse.get(entity.index())?;
#[cfg(debug_assertions)]
assert_eq!(entity, self.entities[dense_index]);
assert_eq!(entity, self.entities[dense_index.as_usize()]);
// SAFETY: if the sparse index points to something in the dense vec, it exists
unsafe { Some(self.dense.get_ticks_unchecked(TableRow::new(dense_index))) }
unsafe { Some(self.dense.get_ticks_unchecked(dense_index)) }
}

/// Removes the `entity` from this sparse set and returns a pointer to the associated value (if
/// it exists).
#[must_use = "The returned pointer must be used to drop the removed component."]
pub(crate) fn remove_and_forget(&mut self, entity: Entity) -> Option<OwningPtr<'_>> {
self.sparse.remove(entity.index()).map(|dense_index| {
let dense_index = dense_index as usize;
#[cfg(debug_assertions)]
assert_eq!(entity, self.entities[dense_index]);
self.entities.swap_remove(dense_index);
let is_last = dense_index == self.dense.len() - 1;
assert_eq!(entity, self.entities[dense_index.as_usize()]);
self.entities.swap_remove(dense_index.as_usize());
let is_last = dense_index.as_usize() == self.dense.len() - 1;
// SAFETY: dense_index was just removed from `sparse`, which ensures that it is valid
let (value, _) = unsafe {
self.dense
.swap_remove_and_forget_unchecked(TableRow::new(dense_index))
};
let (value, _) = unsafe { self.dense.swap_remove_and_forget_unchecked(dense_index) };
if !is_last {
let swapped_entity = self.entities[dense_index];
let swapped_entity = self.entities[dense_index.as_usize()];
#[cfg(not(debug_assertions))]
let index = swapped_entity;
#[cfg(debug_assertions)]
let index = swapped_entity.index();
*self.sparse.get_mut(index).unwrap() = dense_index as u32;
*self.sparse.get_mut(index).unwrap() = dense_index;
}
value
})
Expand All @@ -316,20 +301,21 @@ impl ComponentSparseSet {
/// Returns `true` if `entity` had a component value in the sparse set.
pub(crate) fn remove(&mut self, entity: Entity) -> bool {
if let Some(dense_index) = self.sparse.remove(entity.index()) {
let dense_index = dense_index as usize;
#[cfg(debug_assertions)]
assert_eq!(entity, self.entities[dense_index]);
self.entities.swap_remove(dense_index);
let is_last = dense_index == self.dense.len() - 1;
assert_eq!(entity, self.entities[dense_index.as_usize()]);
self.entities.swap_remove(dense_index.as_usize());
let is_last = dense_index.as_usize() == self.dense.len() - 1;
// SAFETY: if the sparse index points to something in the dense vec, it exists
unsafe { self.dense.swap_remove_unchecked(TableRow::new(dense_index)) }
unsafe {
self.dense.swap_remove_unchecked(dense_index);
}
if !is_last {
let swapped_entity = self.entities[dense_index];
let swapped_entity = self.entities[dense_index.as_usize()];
#[cfg(not(debug_assertions))]
let index = swapped_entity;
#[cfg(debug_assertions)]
let index = swapped_entity.index();
*self.sparse.get_mut(index).unwrap() = dense_index as u32;
*self.sparse.get_mut(index).unwrap() = dense_index;
}
true
} else {
Expand Down
Loading