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

Use NonMaxUsize for non-component SparseSets #12083

Merged
merged 2 commits into from
Mar 3, 2024
Merged
Changes from 1 commit
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
Next Next commit
Use nonmax for non-component sparse sets
james7132 committed Feb 24, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 9799f877fff7e7689d85c764a854a8a48cd2ed2c
1 change: 1 addition & 0 deletions crates/bevy_ecs/Cargo.toml
Original file line number Diff line number Diff line change
@@ -28,6 +28,7 @@ rustc-hash = "1.1"
downcast-rs = "1.2"
serde = "1"
thiserror = "1.0"
nonmax = "0.5"

[dev-dependencies]
rand = "0.8"
26 changes: 14 additions & 12 deletions crates/bevy_ecs/src/storage/sparse_set.rs
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ use crate::{
};
use bevy_ptr::{OwningPtr, Ptr};
use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData};
use nonmax::NonMaxUsize;

type EntityIndex = u32;

@@ -335,7 +336,7 @@ impl ComponentSparseSet {
pub struct SparseSet<I, V: 'static> {
dense: Vec<V>,
indices: Vec<I>,
sparse: SparseArray<I, usize>,
sparse: SparseArray<I, NonMaxUsize>,
}

/// A space-optimized version of [`SparseSet`] that cannot be changed
@@ -344,7 +345,7 @@ pub struct SparseSet<I, V: 'static> {
pub(crate) struct ImmutableSparseSet<I, V: 'static> {
dense: Box<[V]>,
indices: Box<[I]>,
sparse: ImmutableSparseArray<I, usize>,
sparse: ImmutableSparseArray<I, NonMaxUsize>,
}

macro_rules! impl_sparse_set {
@@ -368,7 +369,7 @@ macro_rules! impl_sparse_set {
pub fn get(&self, index: I) -> Option<&V> {
self.sparse.get(index).map(|dense_index| {
// SAFETY: 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.get()) }
})
}

@@ -379,7 +380,7 @@ macro_rules! impl_sparse_set {
let dense = &mut self.dense;
self.sparse.get(index).map(move |dense_index| {
// SAFETY: if the sparse index points to something in the dense vec, it exists
unsafe { dense.get_unchecked_mut(*dense_index) }
unsafe { dense.get_unchecked_mut(dense_index.get()) }
})
}

@@ -454,10 +455,10 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
if let Some(dense_index) = self.sparse.get(index.clone()).cloned() {
// SAFETY: dense indices stored in self.sparse always exist
unsafe {
*self.dense.get_unchecked_mut(dense_index) = value;
*self.dense.get_unchecked_mut(dense_index.get()) = value;
}
} else {
self.sparse.insert(index.clone(), self.dense.len());
self.sparse.insert(index.clone(), NonMaxUsize::new(self.dense.len()).unwrap());
self.indices.push(index);
self.dense.push(value);
}
@@ -468,11 +469,11 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
pub fn get_or_insert_with(&mut self, index: I, func: impl FnOnce() -> V) -> &mut V {
if let Some(dense_index) = self.sparse.get(index.clone()).cloned() {
// SAFETY: dense indices stored in self.sparse always exist
unsafe { self.dense.get_unchecked_mut(dense_index) }
unsafe { self.dense.get_unchecked_mut(dense_index.get()) }
} else {
let value = func();
let dense_index = self.dense.len();
self.sparse.insert(index.clone(), dense_index);
self.sparse.insert(index.clone(), NonMaxUsize::new(dense_index).unwrap());
self.indices.push(index);
self.dense.push(value);
// SAFETY: dense index was just populated above
@@ -491,11 +492,12 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
/// Returns `None` if `index` does not have a value in the sparse set.
pub fn remove(&mut self, index: I) -> Option<V> {
self.sparse.remove(index).map(|dense_index| {
let is_last = dense_index == self.dense.len() - 1;
let value = self.dense.swap_remove(dense_index);
self.indices.swap_remove(dense_index);
let index = dense_index.get();
let is_last = index == self.dense.len() - 1;
let value = self.dense.swap_remove(index);
self.indices.swap_remove(index);
if !is_last {
let swapped_index = self.indices[dense_index].clone();
let swapped_index = self.indices[index].clone();
*self.sparse.get_mut(swapped_index).unwrap() = dense_index;
}
value