Mirrors/bevy
2
0
Fork 0
mirror of https://github.com/bevyengine/bevy synced 2024-11-10 15:14:50 +00:00
Code Issues Projects Releases Packages Wiki Activity

Minimize small allocations by dropping the tick Vecs from Resources (#11226)

Browse source 
# Objective
`Column` unconditionally requires three separate allocations: one for
the data, and two for the tick Vecs. The tick Vecs aren't really needed
for Resources, so we're allocating a bunch of one-element Vecs, and it
costs two extra dereferences when fetching/inserting/removing resources.

## Solution
Drop one level lower in `ResourceData` and directly store a `BlobVec`
and two `UnsafeCell<Tick>`s. This should significantly shrink
`ResourceData` (exchanging 6 usizes for 2 u32s), removes the need to
dereference two separate ticks when inserting/removing/fetching
resources, and can significantly decrease the number of small
allocations the ECS makes by default.

This tentatively might have a non-insignificant impact on the CPU cost
for rendering since we're constantly fetching resources in draw
functions, depending on how aggressively inlined the functions are.

This requires reimplementing some of the unsafe functions that `Column`
wraps, but it also allows us to delete a few Column APIs that were only
used for Resources, so the total amount of unsafe we're maintaining
shouldn't change significantly.

---------

Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
This commit is contained in:
James Liu 2024-01-08 14:39:47 -08:00 committed by GitHub
parent bcbb7bb9dd
commit 13570cd4c8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 80 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

109
crates/bevy_ecs/src/storage/resource.rs
View file

@ -1,9 +1,9 @@
use crate::archetype::ArchetypeComponentId;
use crate::change_detection::{MutUntyped, TicksMut};
use crate::component::{ComponentId, ComponentTicks, Components, Tick, TickCells};
use crate::storage::{Column, SparseSet, TableRow};
use crate::storage::{blob_vec::BlobVec, SparseSet};
use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref};
use std::{mem::ManuallyDrop, thread::ThreadId};
use std::{cell::UnsafeCell, mem::ManuallyDrop, thread::ThreadId};
/// The type-erased backing storage and metadata for a single resource within a [`World`].
///
@ -11,7 +11,9 @@ use std::{mem::ManuallyDrop, thread::ThreadId};
///
/// [`World`]: crate::world::World
pub struct ResourceData<const SEND: bool> {
column: ManuallyDrop<Column>,
data: ManuallyDrop<BlobVec>,
added_ticks: UnsafeCell<Tick>,
changed_ticks: UnsafeCell<Tick>,
type_name: String,
id: ArchetypeComponentId,
origin_thread_id: Option<ThreadId>,
@ -33,14 +35,14 @@ impl<const SEND: bool> Drop for ResourceData<SEND> {
// been dropped. The validate_access call above will check that the
// data is dropped on the thread it was inserted from.
unsafe {
ManuallyDrop::drop(&mut self.column);
ManuallyDrop::drop(&mut self.data);
}
}
}
impl<const SEND: bool> ResourceData<SEND> {
/// The only row in the underlying column.
const ROW: TableRow = TableRow::from_u32(0);
/// The only row in the underlying `BlobVec`.
const ROW: usize = 0;
/// Validates the access to `!Send` resources is only done on the thread they were created from.
///
@ -65,7 +67,7 @@ impl<const SEND: bool> ResourceData<SEND> {
/// Returns true if the resource is populated.
#[inline]
pub fn is_present(&self) -> bool {
!self.column.is_empty()
!self.data.is_empty()
}
/// Gets the [`ArchetypeComponentId`] for the resource.
@ -81,16 +83,24 @@ impl<const SEND: bool> ResourceData<SEND> {
/// original thread it was inserted from.
#[inline]
pub fn get_data(&self) -> Option<Ptr<'_>> {
self.column.get_data(Self::ROW).map(|res| {
self.is_present().then(|| {
self.validate_access();
res
// SAFETY: We've already checked if a value is present, and there should only be one.
unsafe { self.data.get_unchecked(Self::ROW) }
})
}
/// Returns a reference to the resource's change ticks, if it exists.
#[inline]
pub fn get_ticks(&self) -> Option<ComponentTicks> {
self.column.get_ticks(Self::ROW)
// SAFETY: This is being fetched through a read-only reference to Self, so no other mutable references
// to the ticks can exist.
unsafe {
self.is_present().then(|| ComponentTicks {
added: self.added_ticks.read(),
changed: self.changed_ticks.read(),
})
}
}
/// Returns references to the resource and its change ticks, if it exists.
@ -100,9 +110,16 @@ impl<const SEND: bool> ResourceData<SEND> {
/// original thread it was inserted in.
#[inline]
pub(crate) fn get_with_ticks(&self) -> Option<(Ptr<'_>, TickCells<'_>)> {
self.column.get(Self::ROW).map(|res| {
self.is_present().then(|| {
self.validate_access();
res
(
// SAFETY: We've already checked if a value is present, and there should only be one.
unsafe { self.data.get_unchecked(Self::ROW) },
TickCells {
added: &self.added_ticks,
changed: &self.changed_ticks,
},
)
})
}
@ -134,13 +151,18 @@ impl<const SEND: bool> ResourceData<SEND> {
pub(crate) unsafe fn insert(&mut self, value: OwningPtr<'_>, change_tick: Tick) {
if self.is_present() {
self.validate_access();
self.column.replace(Self::ROW, value, change_tick);
// SAFETY: The caller ensures that the provided value is valid for the underlying type and
// is properly initialized. We've ensured that a value is already present and previously
// initialized.
self.data.replace_unchecked(Self::ROW, value);
} else {
if !SEND {
self.origin_thread_id = Some(std::thread::current().id());
}
self.column.push(value, ComponentTicks::new(change_tick));
self.data.push(value);
*self.added_ticks.deref_mut() = change_tick;
}
*self.changed_ticks.deref_mut() = change_tick;
}
/// Inserts a value into the resource with a pre-existing change tick. If a
@ -160,18 +182,18 @@ impl<const SEND: bool> ResourceData<SEND> {
) {
if self.is_present() {
self.validate_access();
self.column.replace_untracked(Self::ROW, value);
*self.column.get_added_tick_unchecked(Self::ROW).deref_mut() = change_ticks.added;
*self
.column
.get_changed_tick_unchecked(Self::ROW)
.deref_mut() = change_ticks.changed;
// SAFETY: The caller ensures that the provided value is valid for the underlying type and
// is properly initialized. We've ensured that a value is already present and previously
// initialized.
self.data.replace_unchecked(Self::ROW, value);
} else {
if !SEND {
self.origin_thread_id = Some(std::thread::current().id());
}
self.column.push(value, change_ticks);
self.data.push(value);
}
*self.added_ticks.deref_mut() = change_ticks.added;
*self.changed_ticks.deref_mut() = change_ticks.changed;
}
/// Removes a value from the resource, if present.
@ -182,12 +204,24 @@ impl<const SEND: bool> ResourceData<SEND> {
#[inline]
#[must_use = "The returned pointer to the removed component should be used or dropped"]
pub(crate) fn remove(&mut self) -> Option<(OwningPtr<'_>, ComponentTicks)> {
if SEND {
self.column.swap_remove_and_forget(Self::ROW)
} else {
self.is_present()
.then(|| self.validate_access())
.and_then(|_| self.column.swap_remove_and_forget(Self::ROW))
if !self.is_present() {
return None;
}
if !SEND {
self.validate_access();
}
// SAFETY: We've already validated that the row is present.
let res = unsafe { self.data.swap_remove_and_forget_unchecked(Self::ROW) };
// SAFETY: This function is being called through an exclusive mutable reference to Self, which
// makes it sound to read these ticks.
unsafe {
Some((
res,
ComponentTicks {
added: self.added_ticks.read(),
changed: self.changed_ticks.read(),
},
))
}
}
@ -200,9 +234,14 @@ impl<const SEND: bool> ResourceData<SEND> {
pub(crate) fn remove_and_drop(&mut self) {
if self.is_present() {
self.validate_access();
self.column.clear();
self.data.clear();
}
}
pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) {
self.added_ticks.get_mut().check_tick(change_tick);
self.changed_ticks.get_mut().check_tick(change_tick);
}
}
/// The backing store for all [`Resource`]s stored in the [`World`].
@ -275,8 +314,18 @@ impl<const SEND: bool> Resources<SEND> {
component_info.name(),
);
}
// SAFETY: component_info.drop() is valid for the types that will be inserted.
let data = unsafe {
BlobVec::new(
component_info.layout(),
component_info.drop(),
1
)
};
ResourceData {
column: ManuallyDrop::new(Column::with_capacity(component_info, 1)),
data: ManuallyDrop::new(data),
added_ticks: UnsafeCell::new(Tick::new(0)),
changed_ticks: UnsafeCell::new(Tick::new(0)),
type_name: String::from(component_info.name()),
id: f(),
origin_thread_id: None,
@ -286,7 +335,7 @@ impl<const SEND: bool> Resources<SEND> {
pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) {
for info in self.resources.values_mut() {
info.column.check_change_ticks(change_tick);
info.check_change_ticks(change_tick);
}
}
}

40
crates/bevy_ecs/src/storage/table.rs
View file

@ -204,18 +204,6 @@ impl Column {
.get_mut() = change_tick;
}
/// Writes component data to the column at given row.
/// Assumes the slot is initialized, calls drop.
/// Does not update the Component's ticks.
///
/// # Safety
/// Assumes data has already been allocated for the given row.
#[inline]
pub(crate) unsafe fn replace_untracked(&mut self, row: TableRow, data: OwningPtr<'_>) {
debug_assert!(row.as_usize() < self.len());
self.data.replace_unchecked(row.as_usize(), data);
}
/// Gets the current number of elements stored in the column.
#[inline]
pub fn len(&self) -> usize {
@ -246,33 +234,7 @@ impl Column {
}
/// Removes an element from the [`Column`] and returns it and its change detection ticks.
/// This does not preserve ordering, but is O(1).
///
/// The element is replaced with the last element in the [`Column`].
///
/// It is the caller's responsibility to ensure that the removed value is dropped or used.
/// Failure to do so may result in resources not being released (i.e. files handles not being
/// released, memory leaks, etc.)
///
/// Returns `None` if `row` is out of bounds.
#[inline]
#[must_use = "The returned pointer should be used to drop the removed component"]
pub(crate) fn swap_remove_and_forget(
&mut self,
row: TableRow,
) -> Option<(OwningPtr<'_>, ComponentTicks)> {
(row.as_usize() < self.data.len()).then(|| {
// SAFETY: The row was length checked before this.
let data = unsafe { self.data.swap_remove_and_forget_unchecked(row.as_usize()) };
let added = self.added_ticks.swap_remove(row.as_usize()).into_inner();
let changed = self.changed_ticks.swap_remove(row.as_usize()).into_inner();
(data, ComponentTicks { added, changed })
})
}
/// Removes an element from the [`Column`] and returns it and its change detection ticks.
/// This does not preserve ordering, but is O(1). Unlike [`Column::swap_remove_and_forget`]
/// this does not do any bounds checking.
/// This does not preserve ordering, but is O(1) and does not do any bounds checking.
///
/// The element is replaced with the last element in the [`Column`].
///