Skip to content

Commit 57733bb

Browse files
authored
Use NonMaxUsize for non-component SparseSets (#12083)
# Objective Adoption of #2104 and #11843. The `Option<usize>` wastes 3-7 bytes of memory per potential entry, and represents a scaling memory overhead as the ID space grows. The goal of this PR is to reduce memory usage without significantly impacting common use cases. Co-Authored By: @NathanSWard Co-Authored By: @tygyh ## Solution Replace `usize` in `SparseSet`'s sparse array with `nonmax::NonMaxUsize`. NonMaxUsize wraps a NonZeroUsize, and applies a bitwise NOT to the value when accessing it. This allows the compiler to niche the value and eliminate the extra padding used for the `Option` inside the sparse array, while moving the niche value from 0 to usize::MAX instead. Checking the [diff in x86 generated assembly](james7132/bevy_asm_tests@6e4da65), this change actually results in fewer instructions generated. One potential downside is that it seems to have moved a load before a branch, which means we may be incurring a cache miss even if the element is not there. Note: unlike #2104 and #11843, this PR only targets the metadata stores for the ECS and not the component storage itself. Due to #9907 targeting `Entity::generation` instead of `Entity::index`, `ComponentSparseSet` storing only up to `u32::MAX` elements would become a correctness issue. This will come with a cost when inserting items into the SparseSet, as now there is a potential for a panic. These cost are really only incurred when constructing a new Table, Archetype, or Resource that has never been seen before by the World. All operations that are fairly cold and not on any particular hotpath, even for command application. --- ## Changelog Changed: `SparseSet` now can only store up to `usize::MAX - 1` elements instead of `usize::MAX`. Changed: `SparseSet` now uses 33-50% less memory overhead per stored item.
1 parent 8cf5fbb commit 57733bb

File tree

2 files changed

+17
-12
lines changed

2 files changed

+17
-12
lines changed

crates/bevy_ecs/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ fixedbitset = "0.4.2"
2828
rustc-hash = "1.1"
2929
serde = "1"
3030
thiserror = "1.0"
31+
nonmax = "0.5"
3132

3233
[dev-dependencies]
3334
rand = "0.8"

crates/bevy_ecs/src/storage/sparse_set.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::{
44
storage::{Column, TableRow},
55
};
66
use bevy_ptr::{OwningPtr, Ptr};
7+
use nonmax::NonMaxUsize;
78
use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData};
89

910
type EntityIndex = u32;
@@ -335,7 +336,7 @@ impl ComponentSparseSet {
335336
pub struct SparseSet<I, V: 'static> {
336337
dense: Vec<V>,
337338
indices: Vec<I>,
338-
sparse: SparseArray<I, usize>,
339+
sparse: SparseArray<I, NonMaxUsize>,
339340
}
340341

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

350351
macro_rules! impl_sparse_set {
@@ -368,7 +369,7 @@ macro_rules! impl_sparse_set {
368369
pub fn get(&self, index: I) -> Option<&V> {
369370
self.sparse.get(index).map(|dense_index| {
370371
// SAFETY: if the sparse index points to something in the dense vec, it exists
371-
unsafe { self.dense.get_unchecked(*dense_index) }
372+
unsafe { self.dense.get_unchecked(dense_index.get()) }
372373
})
373374
}
374375

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

@@ -454,10 +455,11 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
454455
if let Some(dense_index) = self.sparse.get(index.clone()).cloned() {
455456
// SAFETY: dense indices stored in self.sparse always exist
456457
unsafe {
457-
*self.dense.get_unchecked_mut(dense_index) = value;
458+
*self.dense.get_unchecked_mut(dense_index.get()) = value;
458459
}
459460
} else {
460-
self.sparse.insert(index.clone(), self.dense.len());
461+
self.sparse
462+
.insert(index.clone(), NonMaxUsize::new(self.dense.len()).unwrap());
461463
self.indices.push(index);
462464
self.dense.push(value);
463465
}
@@ -468,11 +470,12 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
468470
pub fn get_or_insert_with(&mut self, index: I, func: impl FnOnce() -> V) -> &mut V {
469471
if let Some(dense_index) = self.sparse.get(index.clone()).cloned() {
470472
// SAFETY: dense indices stored in self.sparse always exist
471-
unsafe { self.dense.get_unchecked_mut(dense_index) }
473+
unsafe { self.dense.get_unchecked_mut(dense_index.get()) }
472474
} else {
473475
let value = func();
474476
let dense_index = self.dense.len();
475-
self.sparse.insert(index.clone(), dense_index);
477+
self.sparse
478+
.insert(index.clone(), NonMaxUsize::new(dense_index).unwrap());
476479
self.indices.push(index);
477480
self.dense.push(value);
478481
// SAFETY: dense index was just populated above
@@ -491,11 +494,12 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
491494
/// Returns `None` if `index` does not have a value in the sparse set.
492495
pub fn remove(&mut self, index: I) -> Option<V> {
493496
self.sparse.remove(index).map(|dense_index| {
494-
let is_last = dense_index == self.dense.len() - 1;
495-
let value = self.dense.swap_remove(dense_index);
496-
self.indices.swap_remove(dense_index);
497+
let index = dense_index.get();
498+
let is_last = index == self.dense.len() - 1;
499+
let value = self.dense.swap_remove(index);
500+
self.indices.swap_remove(index);
497501
if !is_last {
498-
let swapped_index = self.indices[dense_index].clone();
502+
let swapped_index = self.indices[index].clone();
499503
*self.sparse.get_mut(swapped_index).unwrap() = dense_index;
500504
}
501505
value

0 commit comments

Comments
 (0)