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

[Merged by Bors] - Use u32 over usize for ComponentSparseSet indicies #4723

Closed
wants to merge 3 commits into from
Closed
Changes from 2 commits
Commits
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
42 changes: 23 additions & 19 deletions crates/bevy_ecs/src/storage/sparse_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use crate::{
use bevy_ptr::{OwningPtr, Ptr};
use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData};

type EntityId = u32;

#[derive(Debug)]
pub struct SparseArray<I, V = I> {
values: Vec<Option<V>>,
Expand Down Expand Up @@ -96,8 +98,8 @@ impl<I: SparseSetIndex, V> SparseArray<I, V> {
pub struct ComponentSparseSet {
james7132 marked this conversation as resolved.
Show resolved Hide resolved
dense: BlobVec,
ticks: Vec<UnsafeCell<ComponentTicks>>,
entities: Vec<Entity>,
sparse: SparseArray<Entity, usize>,
entities: Vec<EntityId>,
james7132 marked this conversation as resolved.
Show resolved Hide resolved
sparse: SparseArray<EntityId, u32>,
}

impl ComponentSparseSet {
Expand Down Expand Up @@ -137,82 +139,84 @@ impl ComponentSparseSet {
/// The `value` pointer must point to a valid address that matches the [`Layout`](std::alloc::Layout)
/// inside the [`ComponentInfo`] given when constructing this sparse set.
pub unsafe fn insert(&mut self, entity: Entity, value: OwningPtr<'_>, change_tick: u32) {
if let Some(&dense_index) = self.sparse.get(entity) {
self.dense.replace_unchecked(dense_index, value);
*self.ticks.get_unchecked_mut(dense_index) =
if let Some(&dense_index) = self.sparse.get(entity.id()) {
self.dense.replace_unchecked(dense_index as usize, value);
james7132 marked this conversation as resolved.
Show resolved Hide resolved
*self.ticks.get_unchecked_mut(dense_index as usize) =
UnsafeCell::new(ComponentTicks::new(change_tick));
} else {
let dense_index = self.dense.len();
self.dense.push(value);
self.sparse.insert(entity, dense_index);
self.sparse.insert(entity.id(), dense_index as u32);
debug_assert_eq!(self.ticks.len(), dense_index);
debug_assert_eq!(self.entities.len(), dense_index);
self.ticks
.push(UnsafeCell::new(ComponentTicks::new(change_tick)));
self.entities.push(entity);
self.entities.push(entity.id());
}
}

#[inline]
pub fn contains(&self, entity: Entity) -> bool {
self.sparse.contains(entity)
self.sparse.contains(entity.id())
james7132 marked this conversation as resolved.
Show resolved Hide resolved
}

#[inline]
pub fn get(&self, entity: Entity) -> Option<Ptr<'_>> {
self.sparse.get(entity).map(|dense_index| {
self.sparse.get(entity.id()).map(|dense_index| {
// SAFE: if the sparse index points to something in the dense vec, it exists
unsafe { self.dense.get_unchecked(*dense_index) }
unsafe { self.dense.get_unchecked(*dense_index as usize) }
})
}

#[inline]
pub fn get_with_ticks(&self, entity: Entity) -> Option<(Ptr<'_>, &UnsafeCell<ComponentTicks>)> {
let dense_index = *self.sparse.get(entity)?;
let dense_index = *self.sparse.get(entity.id())?;
// SAFE: if the sparse index points to something in the dense vec, it exists
unsafe {
Some((
self.dense.get_unchecked(dense_index),
self.ticks.get_unchecked(dense_index),
self.dense.get_unchecked(dense_index as usize),
self.ticks.get_unchecked(dense_index as usize),
))
}
}

#[inline]
pub fn get_ticks(&self, entity: Entity) -> Option<&UnsafeCell<ComponentTicks>> {
let dense_index = *self.sparse.get(entity)?;
let dense_index = *self.sparse.get(entity.id())?;
// SAFE: if the sparse index points to something in the dense vec, it exists
unsafe { Some(self.ticks.get_unchecked(dense_index)) }
unsafe { Some(self.ticks.get_unchecked(dense_index as usize)) }
}

/// 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 fn remove_and_forget(&mut self, entity: Entity) -> Option<OwningPtr<'_>> {
self.sparse.remove(entity).map(|dense_index| {
self.sparse.remove(entity.id()).map(|dense_index| {
let dense_index = dense_index as usize;
self.ticks.swap_remove(dense_index);
self.entities.swap_remove(dense_index);
let is_last = dense_index == self.dense.len() - 1;
// SAFE: dense_index was just removed from `sparse`, which ensures that it is valid
let value = unsafe { self.dense.swap_remove_and_forget_unchecked(dense_index) };
if !is_last {
let swapped_entity = self.entities[dense_index];
*self.sparse.get_mut(swapped_entity).unwrap() = dense_index;
*self.sparse.get_mut(swapped_entity).unwrap() = dense_index as u32;
}
value
})
}

pub fn remove(&mut self, entity: Entity) -> bool {
if let Some(dense_index) = self.sparse.remove(entity) {
if let Some(dense_index) = self.sparse.remove(entity.id()) {
let dense_index = dense_index as usize;
self.ticks.swap_remove(dense_index);
self.entities.swap_remove(dense_index);
let is_last = dense_index == self.dense.len() - 1;
// SAFE: if the sparse index points to something in the dense vec, it exists
unsafe { self.dense.swap_remove_and_drop_unchecked(dense_index) }
if !is_last {
let swapped_entity = self.entities[dense_index];
*self.sparse.get_mut(swapped_entity).unwrap() = dense_index;
*self.sparse.get_mut(swapped_entity).unwrap() = dense_index as u32;
}
true
} else {
Expand Down