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] - Immutable sparse sets for metadata storage #4928

Closed
wants to merge 15 commits into from
Closed
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
bundle::BundleId,
component::{ComponentId, StorageType},
entity::{Entity, EntityLocation},
storage::{SparseArray, SparseSet, SparseSetIndex, TableId},
storage::{ImmutableSparseSet, SparseArray, SparseSet, SparseSetIndex, TableId},
};
use std::{
collections::HashMap,
Expand Down Expand Up @@ -153,7 +153,7 @@ pub struct Archetype {
entities: Vec<ArchetypeEntity>,
table_components: Box<[ComponentId]>,
sparse_set_components: Box<[ComponentId]>,
components: SparseSet<ComponentId, ArchetypeComponentInfo>,
components: ImmutableSparseSet<ComponentId, ArchetypeComponentInfo>,
}

impl Archetype {
Expand Down Expand Up @@ -195,7 +195,7 @@ impl Archetype {
id,
table_id,
entities: Vec::new(),
components,
components: components.into_immutable(),
table_components,
sparse_set_components,
edges: Default::default(),
Expand Down
153 changes: 102 additions & 51 deletions crates/bevy_ecs/src/storage/sparse_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ pub(crate) struct SparseArray<I, V = I> {
marker: PhantomData<I>,
}

/// A space-optimized version of [`SparseArray`] that cannot be changed
/// after contrusction.
james7132 marked this conversation as resolved.
Show resolved Hide resolved
#[derive(Debug)]
pub(crate) struct ImmutableSparseArray<I, V = I> {
values: Box<[Option<V>]>,
marker: PhantomData<I>,
}

impl<I: SparseSetIndex, V> Default for SparseArray<I, V> {
fn default() -> Self {
Self::new()
Expand All @@ -30,6 +38,27 @@ impl<I, V> SparseArray<I, V> {
}
}

macro_rules! impl_sparse_array {
($ty:ident) => {
impl<I: SparseSetIndex, V> $ty<I, V> {
#[inline]
pub fn contains(&self, index: I) -> bool {
let index = index.sparse_set_index();
self.values.get(index).map(|v| v.is_some()).unwrap_or(false)
}

#[inline]
pub fn get(&self, index: I) -> Option<&V> {
let index = index.sparse_set_index();
self.values.get(index).map(|v| v.as_ref()).unwrap_or(None)
}
}
};
}

impl_sparse_array!(SparseArray);
impl_sparse_array!(ImmutableSparseArray);

impl<I: SparseSetIndex, V> SparseArray<I, V> {
#[inline]
pub fn insert(&mut self, index: I, value: V) {
Expand All @@ -40,18 +69,6 @@ impl<I: SparseSetIndex, V> SparseArray<I, V> {
self.values[index] = Some(value);
}

#[inline]
pub fn contains(&self, index: I) -> bool {
let index = index.sparse_set_index();
self.values.get(index).map(|v| v.is_some()).unwrap_or(false)
}

#[inline]
pub fn get(&self, index: I) -> Option<&V> {
let index = index.sparse_set_index();
self.values.get(index).map(|v| v.as_ref()).unwrap_or(None)
}

#[inline]
pub fn get_mut(&mut self, index: I) -> Option<&mut V> {
let index = index.sparse_set_index();
Expand All @@ -70,6 +87,13 @@ impl<I: SparseSetIndex, V> SparseArray<I, V> {
pub fn clear(&mut self) {
self.values.clear();
}

pub(crate) fn into_immutable(self) -> ImmutableSparseArray<I, V> {
ImmutableSparseArray {
values: self.values.into_boxed_slice(),
marker: PhantomData,
}
}
}

/// A sparse data structure of [Components](crate::component::Component)
Expand Down Expand Up @@ -249,11 +273,71 @@ pub struct SparseSet<I, V: 'static> {
sparse: SparseArray<I, usize>,
}

/// A space-optimized version of [`SparseSet`] that cannot be changed
/// after contrusction.
james7132 marked this conversation as resolved.
Show resolved Hide resolved
#[derive(Debug)]
pub(crate) struct ImmutableSparseSet<I, V: 'static> {
dense: Box<[V]>,
indices: Box<[I]>,
sparse: ImmutableSparseArray<I, usize>,
}

macro_rules! impl_sparse_set {
($ty:ident) => {
impl<I: SparseSetIndex, V> $ty<I, V> {
#[inline]
pub fn len(&self) -> usize {
self.dense.len()
}

#[inline]
pub fn contains(&self, index: I) -> bool {
self.sparse.contains(index)
}

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) }
})
}

pub fn get_mut(&mut self, index: I) -> Option<&mut V> {
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) }
})
}

pub fn indices(&self) -> impl Iterator<Item = I> + '_ {
self.indices.iter().cloned()
}

pub fn values(&self) -> impl Iterator<Item = &V> {
self.dense.iter()
}

pub fn values_mut(&mut self) -> impl Iterator<Item = &mut V> {
self.dense.iter_mut()
}

pub fn iter_mut(&mut self) -> impl Iterator<Item = (&I, &mut V)> {
self.indices.iter().zip(self.dense.iter_mut())
}
}
};
}

impl_sparse_set!(SparseSet);
impl_sparse_set!(ImmutableSparseSet);

impl<I: SparseSetIndex, V> Default for SparseSet<I, V> {
fn default() -> Self {
Self::new()
}
}

impl<I, V> SparseSet<I, V> {
pub const fn new() -> Self {
Self {
Expand Down Expand Up @@ -306,36 +390,11 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
}
}

#[inline]
pub fn len(&self) -> usize {
self.dense.len()
}

#[inline]
pub fn is_empty(&self) -> bool {
self.dense.len() == 0
}

#[inline]
pub fn contains(&self, index: I) -> bool {
self.sparse.contains(index)
}

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) }
})
}

pub fn get_mut(&mut self, index: I) -> Option<&mut V> {
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) }
})
}

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;
Expand All @@ -349,24 +408,16 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
})
}

pub fn indices(&self) -> impl Iterator<Item = I> + '_ {
self.indices.iter().cloned()
}

pub fn values(&self) -> impl Iterator<Item = &V> {
self.dense.iter()
}

pub fn values_mut(&mut self) -> impl Iterator<Item = &mut V> {
self.dense.iter_mut()
}

pub fn iter(&self) -> impl Iterator<Item = (&I, &V)> {
self.indices.iter().zip(self.dense.iter())
}

pub fn iter_mut(&mut self) -> impl Iterator<Item = (&I, &mut V)> {
self.indices.iter().zip(self.dense.iter_mut())
pub(crate) fn into_immutable(self) -> ImmutableSparseSet<I, V> {
ImmutableSparseSet {
dense: self.dense.into_boxed_slice(),
indices: self.indices.into_boxed_slice(),
sparse: self.sparse.into_immutable(),
}
}
}

Expand Down
81 changes: 57 additions & 24 deletions crates/bevy_ecs/src/storage/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::{
component::{ComponentId, ComponentInfo, ComponentTicks, Components},
entity::Entity,
query::debug_checked_unreachable,
storage::{blob_vec::BlobVec, SparseSet},
storage::{blob_vec::BlobVec, ImmutableSparseSet, SparseSet},
};
use bevy_ptr::{OwningPtr, Ptr, PtrMut};
use bevy_utils::HashMap;
Expand Down Expand Up @@ -262,31 +262,68 @@ impl Column {
}
}

pub struct Table {
/// A builder type for constructing [`Table`]s.
///
/// - Use [`with_capacity`] to initialize the builder.
/// - Repeatedly call [`add_column`] to add columns for components.
/// - Finalize with [`build`] to get the constructed [`Table`].
///
/// [`with_capacity`]: Self::with_capacity
/// [`add_column`]: Self::add_column
/// [`build`]: Self::build
pub(crate) struct TableBuilder {
columns: SparseSet<ComponentId, Column>,
entities: Vec<Entity>,
capacity: usize,
}

impl Table {
pub(crate) fn with_capacity(capacity: usize, column_capacity: usize) -> Table {
impl TableBuilder {
/// Creates a blank [`Table`], allocating space for `column_capacity` columns
/// with the capacity to hold `capacity` entities worth of components each.
pub fn with_capacity(capacity: usize, column_capacity: usize) -> Self {
james7132 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a chance that we end up realloc-ing our tables on construction in some / all cases, due to the fact that with_capacity isn't guaranteed to allocate exactly capacity, but shrink_to_fit will shrink to capacity?

Copy link
Member Author

@james7132 james7132 Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is, but unless we're constantly making new Tables and Archetypes, the cost IMO is negligible over the lifetime of the app.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The frequency might go up drastically if we start doing "fragmenting relations". In that context the bytes saved might not be worth it. Hard to say what to value more.

I guess if/when we implement fragmenting relations we can let benchmarks decide. But this level of nuance is easy to forget about.

Self {
columns: SparseSet::with_capacity(column_capacity),
entities: Vec::with_capacity(capacity),
capacity,
}
}

#[inline]
pub fn entities(&self) -> &[Entity] {
&self.entities
}

pub(crate) fn add_column(&mut self, component_info: &ComponentInfo) {
pub fn add_column(&mut self, component_info: &ComponentInfo) {
self.columns.insert(
component_info.id(),
Column::with_capacity(component_info, self.entities.capacity()),
Column::with_capacity(component_info, self.capacity),
);
}

pub fn build(self) -> Table {
Table {
columns: self.columns.into_immutable(),
entities: Vec::with_capacity(self.capacity),
}
}
}

/// A column-oriented [structure-of-arrays] based storage for [`Component`]s of entities
/// in a [`World`].
///
/// Conceptually, a `Table` can be thought of as an `HashMap<ComponentId, Column>`, where
/// each `Column` is a type-erased `Vec<T: Component>`. Each row corresponds to a single entity
/// (i.e. index 3 in Column A and index 3 in Column B point to different components on the same
/// entity). Fetching components from a table involves fetching the associated column for a
/// component type (via it's [`ComponentId`]), then fetching the entity's row within that column.
///
/// [structure-of-arrays]: https://en.wikipedia.org/wiki/AoS_and_SoA#Structure_of_arrays
/// [`Component`]: crate::component::Component
/// [`World`]: crate::world::World
pub struct Table {
james7132 marked this conversation as resolved.
Show resolved Hide resolved
columns: ImmutableSparseSet<ComponentId, Column>,
entities: Vec<Entity>,
}

impl Table {
#[inline]
pub fn entities(&self) -> &[Entity] {
&self.entities
}

/// Removes the entity at the given row and returns the entity swapped in to replace it (if an
/// entity was swapped in)
///
Expand Down Expand Up @@ -457,11 +494,6 @@ impl Table {
self.entities.capacity()
}

#[inline]
pub fn component_capacity(&self) -> usize {
self.columns.capacity()
}

#[inline]
pub fn is_empty(&self) -> bool {
self.entities.is_empty()
Expand Down Expand Up @@ -495,7 +527,7 @@ pub struct Tables {

impl Default for Tables {
fn default() -> Self {
let empty_table = Table::with_capacity(0, 0);
let empty_table = TableBuilder::with_capacity(0, 0).build();
Tables {
tables: vec![empty_table],
table_ids: HashMap::default(),
Expand Down Expand Up @@ -548,11 +580,11 @@ impl Tables {
.raw_entry_mut()
.from_key(component_ids)
.or_insert_with(|| {
let mut table = Table::with_capacity(0, component_ids.len());
let mut table = TableBuilder::with_capacity(0, component_ids.len());
for component_id in component_ids {
table.add_column(components.get_info_unchecked(*component_id));
}
tables.push(table);
tables.push(table.build());
(component_ids.to_vec(), TableId(tables.len() - 1))
});

Expand Down Expand Up @@ -601,7 +633,7 @@ mod tests {
use crate::{
component::{ComponentTicks, Components},
entity::Entity,
storage::Table,
storage::TableBuilder,
};
#[derive(Component)]
struct W<T>(T);
Expand All @@ -612,8 +644,9 @@ mod tests {
let mut storages = Storages::default();
let component_id = components.init_component::<W<usize>>(&mut storages);
let columns = &[component_id];
let mut table = Table::with_capacity(0, columns.len());
table.add_column(components.get_info(component_id).unwrap());
let mut builder = TableBuilder::with_capacity(0, columns.len());
builder.add_column(components.get_info(component_id).unwrap());
let mut table = builder.build();
let entities = (0..200).map(Entity::from_raw).collect::<Vec<_>>();
for entity in &entities {
// SAFETY: we allocate and immediately set data afterwards
Expand Down