Skip to content

Commit

Permalink
Extract Resources into their own dedicated storage (bevyengine#4809)
Browse files Browse the repository at this point in the history
# Objective
At least partially addresses bevyengine#6282.

Resources are currently stored as a dedicated Resource archetype (ID 1). This allows for easy code reusability, but unnecessarily adds 72 bytes (on 64-bit systems) to the struct that is only used for that one archetype. It also requires several fields to be `pub(crate)` which isn't ideal.

This should also remove one sparse-set lookup from fetching, inserting, and removing resources from a `World`.

## Solution

- Add `Resources` parallel to `Tables` and `SparseSets` and extract the functionality used by `Archetype` in it.
- Remove `unique_components` from `Archetype`
- Remove the `pub(crate)` on `Archetype::components`.
- Remove `ArchetypeId::RESOURCE`
- Remove `Archetypes::resource` and `Archetypes::resource_mut`

---

## Changelog
Added: `Resources` type to store resources.
Added: `Storages::resource`
Removed: `ArchetypeId::RESOURCE`
Removed: `Archetypes::resource` and `Archetypes::resources`
Removed: `Archetype::unique_components` and `Archetypes::unique_components_mut`

## Migration Guide
Resources have been moved to `Resources` under `Storages` in `World`. All code dependent on `Archetype::unique_components(_mut)` should access it via `world.storages().resources()` instead.

All APIs accessing the raw data of individual resources (mutable *and* read-only) have been removed as these APIs allowed for unsound unsafe code. All usages of these APIs should be changed to use `World::{get, insert, remove}_resource`.
  • Loading branch information
james7132 authored and Pietrek14 committed Dec 17, 2022
1 parent eeae4e9 commit 84de928
Show file tree
Hide file tree
Showing 8 changed files with 389 additions and 248 deletions.
43 changes: 2 additions & 41 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::{Column, SparseArray, SparseSet, SparseSetIndex, TableId},
storage::{SparseArray, SparseSet, SparseSetIndex, TableId},
};
use std::{
collections::HashMap,
Expand All @@ -18,7 +18,6 @@ pub struct ArchetypeId(usize);

impl ArchetypeId {
pub const EMPTY: ArchetypeId = ArchetypeId(0);
pub const RESOURCE: ArchetypeId = ArchetypeId(1);
pub const INVALID: ArchetypeId = ArchetypeId(usize::MAX);

#[inline]
Expand Down Expand Up @@ -140,8 +139,7 @@ pub struct Archetype {
table_info: TableInfo,
table_components: Box<[ComponentId]>,
sparse_set_components: Box<[ComponentId]>,
pub(crate) unique_components: SparseSet<ComponentId, Column>,
pub(crate) components: SparseSet<ComponentId, ArchetypeComponentInfo>,
components: SparseSet<ComponentId, ArchetypeComponentInfo>,
}

impl Archetype {
Expand Down Expand Up @@ -188,7 +186,6 @@ impl Archetype {
components,
table_components,
sparse_set_components,
unique_components: SparseSet::new(),
entities: Default::default(),
edges: Default::default(),
}
Expand Down Expand Up @@ -224,16 +221,6 @@ impl Archetype {
&self.sparse_set_components
}

#[inline]
pub fn unique_components(&self) -> &SparseSet<ComponentId, Column> {
&self.unique_components
}

#[inline]
pub fn unique_components_mut(&mut self) -> &mut SparseSet<ComponentId, Column> {
&mut self.unique_components
}

#[inline]
pub fn components(&self) -> impl Iterator<Item = ComponentId> + '_ {
self.components.indices()
Expand Down Expand Up @@ -392,17 +379,6 @@ impl Default for Archetypes {
archetype_component_count: 0,
};
archetypes.get_id_or_insert(TableId::empty(), Vec::new(), Vec::new());

// adds the resource archetype. it is "special" in that it is inaccessible via a "hash",
// which prevents entities from being added to it
archetypes.archetypes.push(Archetype::new(
ArchetypeId::RESOURCE,
TableId::empty(),
Box::new([]),
Box::new([]),
Vec::new(),
Vec::new(),
));
archetypes
}
}
Expand Down Expand Up @@ -433,21 +409,6 @@ impl Archetypes {
}
}

#[inline]
pub fn resource(&self) -> &Archetype {
// SAFETY: resource archetype always exists
unsafe { self.archetypes.get_unchecked(ArchetypeId::RESOURCE.index()) }
}

#[inline]
pub(crate) fn resource_mut(&mut self) -> &mut Archetype {
// SAFETY: resource archetype always exists
unsafe {
self.archetypes
.get_unchecked_mut(ArchetypeId::RESOURCE.index())
}
}

#[inline]
pub fn is_empty(&self) -> bool {
self.archetypes.is_empty()
Expand Down
13 changes: 3 additions & 10 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -956,11 +956,7 @@ mod tests {
.components()
.get_resource_id(TypeId::of::<Num>())
.unwrap();
let archetype_component_id = world
.archetypes()
.resource()
.get_archetype_component_id(resource_id)
.unwrap();
let archetype_component_id = world.storages().resources.get(resource_id).unwrap().id();

assert_eq!(world.resource::<Num>().0, 123);
assert!(world.contains_resource::<Num>());
Expand Down Expand Up @@ -1023,11 +1019,8 @@ mod tests {
"resource id does not change after removing / re-adding"
);

let current_archetype_component_id = world
.archetypes()
.resource()
.get_archetype_component_id(current_resource_id)
.unwrap();
let current_archetype_component_id =
world.storages().resources.get(resource_id).unwrap().id();

assert_eq!(
archetype_component_id, current_archetype_component_id,
Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_ecs/src/storage/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
//! Storage layouts for ECS data.

mod blob_vec;
mod resource;
mod sparse_set;
mod table;

pub use resource::*;
pub use sparse_set::*;
pub use table::*;

Expand All @@ -12,4 +14,5 @@ pub use table::*;
pub struct Storages {
pub sparse_sets: SparseSets,
pub tables: Tables,
pub resources: Resources,
}
185 changes: 185 additions & 0 deletions crates/bevy_ecs/src/storage/resource.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
use crate::archetype::ArchetypeComponentId;
use crate::component::{ComponentId, ComponentTicks, Components};
use crate::storage::{Column, SparseSet};
use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref};
use std::cell::UnsafeCell;

/// The type-erased backing storage and metadata for a single resource within a [`World`].
///
/// [`World`]: crate::world::World
pub struct ResourceData {
column: Column,
id: ArchetypeComponentId,
}

impl ResourceData {
/// Returns true if the resource is populated.
#[inline]
pub fn is_present(&self) -> bool {
!self.column.is_empty()
}

/// Gets the [`ArchetypeComponentId`] for the resource.
#[inline]
pub fn id(&self) -> ArchetypeComponentId {
self.id
}

/// Gets a read-only pointer to the underlying resource, if available.
#[inline]
pub fn get_data(&self) -> Option<Ptr<'_>> {
self.column.get_data(0)
}

/// Gets a read-only reference to the change ticks of the underlying resource, if available.
#[inline]
pub fn get_ticks(&self) -> Option<&ComponentTicks> {
self.column
.get_ticks(0)
// SAFETY:
// - This borrow's lifetime is bounded by the lifetime on self.
// - A read-only borrow on self can only exist while a mutable borrow doesn't
// exist.
.map(|ticks| unsafe { ticks.deref() })
}

#[inline]
pub(crate) fn get_with_ticks(&self) -> Option<(Ptr<'_>, &UnsafeCell<ComponentTicks>)> {
self.column.get(0)
}

/// Inserts a value into the resource. If a value is already present
/// it will be replaced.
///
/// # Safety
/// `value` must be valid for the underlying type for the resource.
///
/// The underlying type must be [`Send`] or be inserted from the main thread.
/// This can be validated with [`World::validate_non_send_access_untyped`].
///
/// [`World::validate_non_send_access_untyped`]: crate::world::World::validate_non_send_access_untyped
#[inline]
pub(crate) unsafe fn insert(&mut self, value: OwningPtr<'_>, change_tick: u32) {
if self.is_present() {
self.column.replace(0, value, change_tick);
} else {
self.column.push(value, ComponentTicks::new(change_tick));
}
}

/// Inserts a value into the resource with a pre-existing change tick. If a
/// value is already present it will be replaced.
///
/// # Safety
/// `value` must be valid for the underlying type for the resource.
///
/// The underlying type must be [`Send`] or be inserted from the main thread.
/// This can be validated with [`World::validate_non_send_access_untyped`].
///
/// [`World::validate_non_send_access_untyped`]: crate::world::World::validate_non_send_access_untyped
#[inline]
pub(crate) unsafe fn insert_with_ticks(
&mut self,
value: OwningPtr<'_>,
change_ticks: ComponentTicks,
) {
if self.is_present() {
self.column.replace_untracked(0, value);
*self.column.get_ticks_unchecked(0).deref_mut() = change_ticks;
} else {
self.column.push(value, change_ticks);
}
}

/// Removes a value from the resource, if present.
///
/// # Safety
/// The underlying type must be [`Send`] or be removed from the main thread.
/// This can be validated with [`World::validate_non_send_access_untyped`].
///
/// The removed value must be used or dropped.
///
/// [`World::validate_non_send_access_untyped`]: crate::world::World::validate_non_send_access_untyped
#[inline]
#[must_use = "The returned pointer to the removed component should be used or dropped"]
pub(crate) unsafe fn remove(&mut self) -> Option<(OwningPtr<'_>, ComponentTicks)> {
self.column.swap_remove_and_forget(0)
}

/// Removes a value from the resource, if present, and drops it.
///
/// # Safety
/// The underlying type must be [`Send`] or be removed from the main thread.
/// This can be validated with [`World::validate_non_send_access_untyped`].
///
/// [`World::validate_non_send_access_untyped`]: crate::world::World::validate_non_send_access_untyped
#[inline]
pub(crate) unsafe fn remove_and_drop(&mut self) {
self.column.clear();
}
}

/// The backing store for all [`Resource`]s stored in the [`World`].
///
/// [`Resource`]: crate::system::Resource
/// [`World`]: crate::world::World
#[derive(Default)]
pub struct Resources {
resources: SparseSet<ComponentId, ResourceData>,
}

impl Resources {
/// The total number of resources stored in the [`World`]
///
/// [`World`]: crate::world::World
#[inline]
pub fn len(&self) -> usize {
self.resources.len()
}

/// Returns true if there are no resources stored in the [`World`],
/// false otherwise.
///
/// [`World`]: crate::world::World
#[inline]
pub fn is_empty(&self) -> bool {
self.resources.is_empty()
}

/// Gets read-only access to a resource, if it exists.
#[inline]
pub fn get(&self, component_id: ComponentId) -> Option<&ResourceData> {
self.resources.get(component_id)
}

/// Gets mutable access to a resource, if it exists.
#[inline]
pub(crate) fn get_mut(&mut self, component_id: ComponentId) -> Option<&mut ResourceData> {
self.resources.get_mut(component_id)
}

/// Fetches or initializes a new resource and returns back it's underlying column.
///
/// # Panics
/// Will panic if `component_id` is not valid for the provided `components`
pub(crate) fn initialize_with(
&mut self,
component_id: ComponentId,
components: &Components,
f: impl FnOnce() -> ArchetypeComponentId,
) -> &mut ResourceData {
self.resources.get_or_insert_with(component_id, || {
let component_info = components.get_info(component_id).unwrap();
ResourceData {
column: Column::with_capacity(component_info, 1),
id: f(),
}
})
}

pub(crate) fn check_change_ticks(&mut self, change_tick: u32) {
for info in self.resources.values_mut() {
info.column.check_change_ticks(change_tick);
}
}
}

0 comments on commit 84de928

Please sign in to comment.