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] - Extract Resources into their own dedicated storage #4809

Closed
wants to merge 40 commits into from
Closed
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
7cd3e3f
Preliminary changes
james7132 May 20, 2022
76ede2f
Fix compilation
james7132 May 20, 2022
b7b16be
Provide internal helper function
james7132 May 31, 2022
5464d47
More complete Resources impl
james7132 May 31, 2022
32c5456
Fix build
james7132 May 31, 2022
bb9a49f
Cleanup
james7132 May 31, 2022
b719c1b
Fix CI
james7132 May 31, 2022
99f14d7
Fix CI
james7132 Jun 7, 2022
4918e10
Shrink Resources
james7132 Jun 7, 2022
2b24bac
Less jank way of fetching ticks.
james7132 Jun 11, 2022
ab675d1
Better safety comment.
james7132 Jun 11, 2022
bd25ccc
Better safety comment.
james7132 Jun 11, 2022
70578fe
Correct safety comment pointing to the right parameter.
james7132 Jun 11, 2022
27ce1a3
Much simpler remove impl.
james7132 Jun 11, 2022
520fd30
Update safety comments
james7132 Jun 15, 2022
7737705
Move the unsafe into Column
james7132 Jun 15, 2022
a6b5e4b
Column::get
james7132 Jun 15, 2022
610c617
Formatting
james7132 Jun 15, 2022
e32ec9e
Address World review comments
james7132 Jun 15, 2022
515e2b3
Make storage APIs a bit more externally friendly
james7132 Jun 15, 2022
c1f1133
Remove contains
james7132 Jun 15, 2022
850b318
Merge branch 'main' into no-unique-components
james7132 Jun 16, 2022
a0731e6
Merge branch 'main' into no-unique-components
james7132 Jul 2, 2022
f4123b9
Fix test compilation
james7132 Jul 2, 2022
bb38531
Fix must_use typo
james7132 Oct 13, 2022
bb59857
Change panic message.
james7132 Oct 13, 2022
3556580
ResourceInfo -> ResourceData
james7132 Oct 13, 2022
734584d
Type doc comment for ResourceData
james7132 Oct 13, 2022
01c176f
Merge branch 'main' into no-unique-components
james7132 Oct 15, 2022
58027c4
Fix CI
james7132 Oct 16, 2022
d3d846a
Update get's doc comment.
james7132 Oct 16, 2022
40afd47
Update get_mut's doc comment.
james7132 Oct 16, 2022
7b9fbf0
Fix docs typo
james7132 Oct 17, 2022
a6c8977
Use an impl param instead of a bound
james7132 Oct 17, 2022
39be822
ArchetypeComponentInfo -> Id
james7132 Oct 17, 2022
efd5113
Remove public access to data methods
james7132 Oct 18, 2022
535bc9f
Update safety comments on mutative resource APIs
james7132 Oct 18, 2022
7932fa8
Fix CI
james7132 Oct 18, 2022
c2476ed
Update safety docs on get_ticks
james7132 Oct 18, 2022
31cd2b4
Formatting
james7132 Oct 18, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 2 additions & 41 deletions crates/bevy_ecs/src/archetype.rs
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>,
james7132 marked this conversation as resolved.
Show resolved Hide resolved
}

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
20 changes: 12 additions & 8 deletions crates/bevy_ecs/src/lib.rs
Expand Up @@ -957,10 +957,12 @@ mod tests {
.get_resource_id(TypeId::of::<Num>())
.unwrap();
let archetype_component_id = world
.archetypes()
.resource()
.get_archetype_component_id(resource_id)
.unwrap();
.storages()
.resources
.get(resource_id)
.unwrap()
.component_info()
.archetype_component_id;

assert_eq!(world.resource::<Num>().0, 123);
assert!(world.contains_resource::<Num>());
Expand Down Expand Up @@ -1024,10 +1026,12 @@ mod tests {
);

let current_archetype_component_id = world
.archetypes()
.resource()
.get_archetype_component_id(current_resource_id)
.unwrap();
.storages()
.resources
.get(resource_id)
.unwrap()
.component_info()
.archetype_component_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
@@ -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,
}
174 changes: 174 additions & 0 deletions crates/bevy_ecs/src/storage/resource.rs
@@ -0,0 +1,174 @@
use crate::archetype::ArchetypeComponentId;
use crate::archetype::ArchetypeComponentInfo;
use crate::component::{ComponentId, ComponentTicks, Components, StorageType};
use crate::storage::{Column, SparseSet};
use bevy_ptr::{OwningPtr, Ptr, PtrMut, 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,
james7132 marked this conversation as resolved.
Show resolved Hide resolved
component_info: ArchetypeComponentInfo,
james7132 marked this conversation as resolved.
Show resolved Hide resolved
}

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

#[inline]
pub(crate) fn component_info(&self) -> &ArchetypeComponentInfo {
&self.component_info
}

/// 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 mutable pointer to the underlying resource, if available.
#[inline]
pub fn get_data_mut(&mut self) -> Option<PtrMut<'_>> {
self.column.get_data_mut(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: If the first row exists, a valid ticks value has been written.
Copy link
Member

Choose a reason for hiding this comment

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

this safety comment has nothing to do with the unsafe block, the unsafe { ticks.deref() } is about whether there are any existing exclusive borrows of ticks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Does this look better?

.map(|ticks| unsafe { ticks.deref() })
}

/// Gets a mutable reference to the change ticks of the underlying resource, if available.
#[inline]
pub fn get_ticks_mut(&mut self) -> Option<&mut ComponentTicks> {
self.column
.get_ticks(0)
// SAFETY: If the first row exists, a valid ticks value has been written.
// This function has exclusvie access to the underlying column.
.map(|ticks| unsafe { ticks.deref_mut() })
}

#[inline]
pub(crate) fn get_with_ticks(&self) -> Option<(Ptr<'_>, &UnsafeCell<ComponentTicks>)> {
james7132 marked this conversation as resolved.
Show resolved Hide resolved
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.
#[inline]
pub unsafe fn insert(&mut self, value: OwningPtr<'_>, change_tick: u32) {
james7132 marked this conversation as resolved.
Show resolved Hide resolved
if self.is_present() {
self.column.replace(0, value, change_tick);
} else {
self.column.push(value, ComponentTicks::new(change_tick));
}
}

#[inline]
pub(crate) unsafe fn insert_with_ticks(
james7132 marked this conversation as resolved.
Show resolved Hide resolved
&mut self,
james7132 marked this conversation as resolved.
Show resolved Hide resolved
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.
#[inline]
#[must_use = "The returned pointer to the removed component should be used or dropped"]
pub fn remove(&mut self) -> Option<(OwningPtr<'_>, ComponentTicks)> {
self.column.swap_remove_and_forget(0)
}

#[inline]
pub(crate) fn remove_and_drop(&mut self) {
james7132 marked this conversation as resolved.
Show resolved Hide resolved
self.column.clear();
james7132 marked this conversation as resolved.
Show resolved Hide resolved
}
}

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

impl Resources {
/// The total number of resoruces stored in the [`World`]
james7132 marked this conversation as resolved.
Show resolved Hide resolved
///
/// [`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 a read-only [`Ptr`] to a resource, if available.
james7132 marked this conversation as resolved.
Show resolved Hide resolved
#[inline]
pub fn get(&self, component_id: ComponentId) -> Option<&ResourceData> {
self.resources.get(component_id)
}

/// Gets a read-only [`Ptr`] to a resource, if available.
james7132 marked this conversation as resolved.
Show resolved Hide resolved
#[inline]
pub fn get_mut(&mut self, component_id: ComponentId) -> Option<&mut ResourceData> {
james7132 marked this conversation as resolved.
Show resolved Hide resolved
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 fn initialize_with<F>(
&mut self,
component_id: ComponentId,
components: &Components,
f: F,
) -> &mut ResourceData
where
F: FnOnce() -> ArchetypeComponentId,
james7132 marked this conversation as resolved.
Show resolved Hide resolved
{
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),
component_info: ArchetypeComponentInfo {
archetype_component_id: f(),
storage_type: StorageType::Table,
},
}
})
}

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