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

Naive garbage collection (Adopted) #11582

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions crates/bevy_ecs/src/archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,13 @@ impl Archetype {
.map(|info| info.archetype_component_id)
}

pub(crate) fn shrink_to_fit(&mut self) {
self.entities.shrink_to_fit();
for component in self.components.values_mut() {
component.shrink_to_fit();
}
}

/// Clears all entities from the archetype.
pub(crate) fn clear_entities(&mut self) {
self.entities.clear();
Expand Down Expand Up @@ -740,6 +747,13 @@ impl Archetypes {
})
}

pub(crate) fn shrink_to_fit(&mut self) {
self.archetypes.shrink_to_fit();
for archetype in self.archetypes.iter_mut() {
archetype.shrink_to_fit();
}
}

/// Returns the number of components that are stored in archetypes.
/// Note that if some component `T` is stored in more than one archetype, it will be counted once for each archetype it's present in.
#[inline]
Expand Down
39 changes: 39 additions & 0 deletions crates/bevy_ecs/src/storage/blob_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,45 @@ impl BlobVec {
std::slice::from_raw_parts(self.data.as_ptr() as *const UnsafeCell<T>, self.len)
}

/// Shrinks the backing storage for the [`BlobVec`] such that the `len == capacity`.
///
/// This runs in `O(n)` time and may require reallocating backing buffer.
pub fn shrink_to_fit(&mut self) {
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a test to this method, since everything else seems to depend on this? Some good things to test:

  • Popping items from a BlobVec then shrinking
  • Shrinking a BlobVec with a capacity of 0
  • Shrinking a BlobVec with a capacity >= 1 but a length of 0

if self.item_layout.size() == 0 || self.len == self.capacity {
return;
}

let current_layout =
array_layout(&self.item_layout, self.capacity).expect("array layout should be valid");
self.data = if self.len == 0 {
// SAFETY:
// - layout has non-zero size as per safety requirement
unsafe {
Copy link
Member

Choose a reason for hiding this comment

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

This needs a safety comment, as does the realloc below. If you need a reference, the safety comments on the allocation functions in grow_exact should be pretty close to the ones we should use here.

std::alloc::dealloc(self.get_ptr_mut().as_ptr(), current_layout);
}
NonNull::dangling()
} else {
// SAFETY:
// - ptr was be allocated via this allocator
// - the layout of the ptr was `array_layout(self.item_layout, self.len)`
// - `item_layout.size() > 0` and `new_capacity > 0`, so the layout size is non-zero
// - "new_size, when rounded up to the nearest multiple of layout.align(), must not underflow (i.e., the rounded value must be more than usize::MIN)",
// since the item size is always a multiple of its align, the rounding cannot happen
// here and the underflow is handled in `array_layout`
let new_layout =
array_layout(&self.item_layout, self.len).expect("array layout should be valid");
let new_data = unsafe {
std::alloc::realloc(
self.get_ptr_mut().as_ptr(),
current_layout,
new_layout.size(),
)
};
NonNull::new(new_data).unwrap_or_else(|| handle_alloc_error(new_layout))
};
self.capacity = self.len;
}

/// Clears the vector, removing (and dropping) all values.
///
/// Note that this method has no effect on the allocated capacity of the vector.
Expand Down
7 changes: 7 additions & 0 deletions crates/bevy_ecs/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,10 @@ pub struct Storages {
/// Backing storage for `!Send` resources.
pub non_send_resources: Resources<false>,
}

impl Storages {
pub(crate) fn shrink_to_fit(&mut self) {
self.tables.shrink_to_fit();
self.sparse_sets.shrink_to_fit();
}
}
12 changes: 12 additions & 0 deletions crates/bevy_ecs/src/storage/sparse_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,12 @@ impl ComponentSparseSet {
}
}

pub fn shrink_to_fit(&mut self) {
Copy link
Member

Choose a reason for hiding this comment

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

This should probably have a doc comment as it's actually usable outside of bevy_ecs.

self.dense.shrink_to_fit();
self.entities.shrink_to_fit();
self.sparse.values.shrink_to_fit();
}

pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) {
self.dense.check_change_ticks(change_tick);
}
Expand Down Expand Up @@ -610,6 +616,12 @@ impl SparseSets {
}
}

pub(crate) fn shrink_to_fit(&mut self) {
for sparse_set in self.sets.values_mut() {
sparse_set.shrink_to_fit();
}
}

pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) {
for set in self.sets.values_mut() {
set.check_change_ticks(change_tick);
Expand Down
26 changes: 26 additions & 0 deletions crates/bevy_ecs/src/storage/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,15 @@ impl Column {
self.changed_ticks.clear();
}

/// Shrinks the backing storage for the [`Column`] such that the `len == capacity`.
///
/// This runs in `O(n)` time and may require reallocating the backing buffers.
pub fn shrink_to_fit(&mut self) {
self.data.shrink_to_fit();
self.added_ticks.shrink_to_fit();
self.changed_ticks.shrink_to_fit();
}

#[inline]
pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) {
for component_ticks in &mut self.added_ticks {
Expand Down Expand Up @@ -782,6 +791,16 @@ impl Table {
self.columns.values()
}

/// Shrinks the backing storage for the [`Table`] such that the `len == capacity`.
///
/// This runs in `O(n)` time and may require reallocating the backing buffers.
pub fn shrink_to_fit(&mut self) {
self.entities.shrink_to_fit();
for column in self.columns.values_mut() {
column.shrink_to_fit();
}
}

/// Clears all of the stored components in the [`Table`].
pub(crate) fn clear(&mut self) {
self.entities.clear();
Expand Down Expand Up @@ -893,6 +912,13 @@ impl Tables {
}
}

pub(crate) fn shrink_to_fit(&mut self) {
self.tables.shrink_to_fit();
for table in &mut self.tables {
table.shrink_to_fit();
}
}

pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) {
for table in &mut self.tables {
table.check_change_ticks(change_tick);
Expand Down
12 changes: 12 additions & 0 deletions crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1889,6 +1889,18 @@ impl World {
self.entities.clear();
}

/// Shrinks the backing storage for the [`World`].
///
/// This can be a very expensive operation on large worlds. Runs in `O(n)` time in both
/// the number of archetypes, tables, and entities stored in the world.
pub fn shrink_to_fit(&mut self) {
self.archetypes.shrink_to_fit();
self.storages.shrink_to_fit();
for components in self.removed_components {
components.shrink_to_fit();
}
}

/// Clears all resources in this [`World`].
///
/// **Note:** Any resource fetch to this [`World`] will fail unless they are re-initialized,
Expand Down
Loading