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](6e4da653cc
),
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.
This commit is contained in:
parent
8cf5fbbf94
commit
57733bbec3
@ -28,6 +28,7 @@ fixedbitset = "0.4.2"
|
|||||||
rustc-hash = "1.1"
|
rustc-hash = "1.1"
|
||||||
serde = "1"
|
serde = "1"
|
||||||
thiserror = "1.0"
|
thiserror = "1.0"
|
||||||
|
nonmax = "0.5"
|
||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
rand = "0.8"
|
rand = "0.8"
|
||||||
|
@ -4,6 +4,7 @@ use crate::{
|
|||||||
storage::{Column, TableRow},
|
storage::{Column, TableRow},
|
||||||
};
|
};
|
||||||
use bevy_ptr::{OwningPtr, Ptr};
|
use bevy_ptr::{OwningPtr, Ptr};
|
||||||
|
use nonmax::NonMaxUsize;
|
||||||
use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData};
|
use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData};
|
||||||
|
|
||||||
type EntityIndex = u32;
|
type EntityIndex = u32;
|
||||||
@ -335,7 +336,7 @@ impl ComponentSparseSet {
|
|||||||
pub struct SparseSet<I, V: 'static> {
|
pub struct SparseSet<I, V: 'static> {
|
||||||
dense: Vec<V>,
|
dense: Vec<V>,
|
||||||
indices: Vec<I>,
|
indices: Vec<I>,
|
||||||
sparse: SparseArray<I, usize>,
|
sparse: SparseArray<I, NonMaxUsize>,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A space-optimized version of [`SparseSet`] that cannot be changed
|
/// 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> {
|
pub(crate) struct ImmutableSparseSet<I, V: 'static> {
|
||||||
dense: Box<[V]>,
|
dense: Box<[V]>,
|
||||||
indices: Box<[I]>,
|
indices: Box<[I]>,
|
||||||
sparse: ImmutableSparseArray<I, usize>,
|
sparse: ImmutableSparseArray<I, NonMaxUsize>,
|
||||||
}
|
}
|
||||||
|
|
||||||
macro_rules! impl_sparse_set {
|
macro_rules! impl_sparse_set {
|
||||||
@ -368,7 +369,7 @@ macro_rules! impl_sparse_set {
|
|||||||
pub fn get(&self, index: I) -> Option<&V> {
|
pub fn get(&self, index: I) -> Option<&V> {
|
||||||
self.sparse.get(index).map(|dense_index| {
|
self.sparse.get(index).map(|dense_index| {
|
||||||
// SAFETY: if the sparse index points to something in the dense vec, it exists
|
// 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;
|
let dense = &mut self.dense;
|
||||||
self.sparse.get(index).map(move |dense_index| {
|
self.sparse.get(index).map(move |dense_index| {
|
||||||
// SAFETY: if the sparse index points to something in the dense vec, it exists
|
// 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,11 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
|
|||||||
if let Some(dense_index) = self.sparse.get(index.clone()).cloned() {
|
if let Some(dense_index) = self.sparse.get(index.clone()).cloned() {
|
||||||
// SAFETY: dense indices stored in self.sparse always exist
|
// SAFETY: dense indices stored in self.sparse always exist
|
||||||
unsafe {
|
unsafe {
|
||||||
*self.dense.get_unchecked_mut(dense_index) = value;
|
*self.dense.get_unchecked_mut(dense_index.get()) = value;
|
||||||
}
|
}
|
||||||
} else {
|
} 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.indices.push(index);
|
||||||
self.dense.push(value);
|
self.dense.push(value);
|
||||||
}
|
}
|
||||||
@ -468,11 +470,12 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
|
|||||||
pub fn get_or_insert_with(&mut self, index: I, func: impl FnOnce() -> V) -> &mut 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() {
|
if let Some(dense_index) = self.sparse.get(index.clone()).cloned() {
|
||||||
// SAFETY: dense indices stored in self.sparse always exist
|
// 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 {
|
} else {
|
||||||
let value = func();
|
let value = func();
|
||||||
let dense_index = self.dense.len();
|
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.indices.push(index);
|
||||||
self.dense.push(value);
|
self.dense.push(value);
|
||||||
// SAFETY: dense index was just populated above
|
// SAFETY: dense index was just populated above
|
||||||
@ -491,11 +494,12 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
|
|||||||
/// Returns `None` if `index` does not have a value in the sparse set.
|
/// Returns `None` if `index` does not have a value in the sparse set.
|
||||||
pub fn remove(&mut self, index: I) -> Option<V> {
|
pub fn remove(&mut self, index: I) -> Option<V> {
|
||||||
self.sparse.remove(index).map(|dense_index| {
|
self.sparse.remove(index).map(|dense_index| {
|
||||||
let is_last = dense_index == self.dense.len() - 1;
|
let index = dense_index.get();
|
||||||
let value = self.dense.swap_remove(dense_index);
|
let is_last = index == self.dense.len() - 1;
|
||||||
self.indices.swap_remove(dense_index);
|
let value = self.dense.swap_remove(index);
|
||||||
|
self.indices.swap_remove(index);
|
||||||
if !is_last {
|
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;
|
*self.sparse.get_mut(swapped_index).unwrap() = dense_index;
|
||||||
}
|
}
|
||||||
value
|
value
|
||||||
|
Loading…
Reference in New Issue
Block a user