diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index f682554ce9a51..bce07be740377 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -417,7 +417,7 @@ impl Archetype { }, ); // NOTE: the `table_components` are sorted AND they were inserted in the `Table` in the same - // sorted order, so the index of the `Column` in the `Table` is the same as the index of the + // sorted order, so the index of the `ThinColumn` in the `Table` is the same as the index of the // component in the `table_components` vector component_index .entry(component_id) diff --git a/crates/bevy_ecs/src/storage/blob_array.rs b/crates/bevy_ecs/src/storage/blob_array.rs index 9b738a763c8ce..221b395af2918 100644 --- a/crates/bevy_ecs/src/storage/blob_array.rs +++ b/crates/bevy_ecs/src/storage/blob_array.rs @@ -1,16 +1,16 @@ -use super::blob_vec::array_layout; -use crate::storage::blob_vec::array_layout_unchecked; use alloc::alloc::handle_alloc_error; use bevy_ptr::{OwningPtr, Ptr, PtrMut}; use bevy_utils::OnDrop; use core::{alloc::Layout, cell::UnsafeCell, num::NonZeroUsize, ptr::NonNull}; -/// A flat, type-erased data storage type similar to a [`BlobVec`](super::blob_vec::BlobVec), but with the length and capacity cut out -/// for performance reasons. This type is reliant on its owning type to store the capacity and length information. +/// A flat, type-erased data storage type. /// /// Used to densely store homogeneous ECS data. A blob is usually just an arbitrary block of contiguous memory without any identity, and -/// could be used to represent any arbitrary data (i.e. string, arrays, etc). This type only stores meta-data about the Blob that it stores, -/// and a pointer to the location of the start of the array, similar to a C array. +/// could be used to represent any arbitrary data (i.e. string, arrays, etc). This type only stores meta-data about the blob that it stores, +/// and a pointer to the location of the start of the array, similar to a C-style `void*` array. +/// +/// for performance reasons. This type is reliant on its owning type to store the capacity and length information. +#[derive(Debug)] pub(super) struct BlobArray { item_layout: Layout, // the `data` ptr's layout is always `array_layout(item_layout, capacity)` @@ -69,10 +69,17 @@ impl BlobArray { self.item_layout.size() == 0 } + /// Returns the drop function for values stored in the vector, + /// or `None` if they don't need to be dropped. + #[inline] + pub fn get_drop(&self) -> Option)> { + self.drop + } + /// Returns a reference to the element at `index`, without doing bounds checking. /// /// *`len` refers to the length of the array, the number of elements that have been initialized, and are safe to read. - /// Just like [`Vec::len`], or [`BlobVec::len`](super::blob_vec::BlobVec::len).* + /// Just like [`Vec::len`].* /// /// # Safety /// - The element at index `index` is safe to access. @@ -95,7 +102,7 @@ impl BlobArray { /// Returns a mutable reference to the element at `index`, without doing bounds checking. /// /// *`len` refers to the length of the array, the number of elements that have been initialized, and are safe to read. - /// Just like [`Vec::len`], or [`BlobVec::len`](super::blob_vec::BlobVec::len).* + /// Just like [`Vec::len`].* /// /// # Safety /// - The element with at index `index` is safe to access. @@ -133,7 +140,7 @@ impl BlobArray { /// To get a slice to the entire array, the caller must plug `len` in `slice_len`. /// /// *`len` refers to the length of the array, the number of elements that have been initialized, and are safe to read. - /// Just like [`Vec::len`], or [`BlobVec::len`](super::blob_vec::BlobVec::len).* + /// Just like [`Vec::len`].* /// /// # Safety /// - The type `T` must be the type of the items in this [`BlobArray`]. @@ -227,6 +234,10 @@ impl BlobArray { /// Allocate a block of memory for the array. This should be used to initialize the array, do not use this /// method if there are already elements stored in the array - use [`Self::realloc`] instead. + /// + /// # Panics + /// - Panics if the new capacity overflows `isize::MAX` bytes. + /// - Panics if the allocation causes an out-of-memory error. pub(super) fn alloc(&mut self, capacity: NonZeroUsize) { #[cfg(debug_assertions)] debug_assert_eq!(self.capacity, 0); @@ -247,6 +258,10 @@ impl BlobArray { /// For example, if the length (number of stored elements) reached the capacity (number of elements the current allocation can store), /// you might want to use this method to increase the allocation, so more data can be stored in the array. /// + /// # Panics + /// - Panics if the new capacity overflows `isize::MAX` bytes. + /// - Panics if the allocation causes an out-of-memory error. + /// /// # Safety /// - `current_capacity` is indeed the current capacity of this array. /// - After calling this method, the caller must update their saved capacity to reflect the change. @@ -477,6 +492,94 @@ impl BlobArray { } } +/// From +pub(super) fn array_layout(layout: &Layout, n: usize) -> Option { + let (array_layout, offset) = repeat_layout(layout, n)?; + debug_assert_eq!(layout.size(), offset); + Some(array_layout) +} + +// TODO: replace with `Layout::repeat` if/when it stabilizes +/// From +fn repeat_layout(layout: &Layout, n: usize) -> Option<(Layout, usize)> { + // This cannot overflow. Quoting from the invariant of Layout: + // > `size`, when rounded up to the nearest multiple of `align`, + // > must not overflow (i.e., the rounded value must be less than + // > `usize::MAX`) + let padded_size = layout.size() + padding_needed_for(layout, layout.align()); + let alloc_size = padded_size.checked_mul(n)?; + + // SAFETY: self.align is already known to be valid and alloc_size has been + // padded already. + unsafe { + Some(( + Layout::from_size_align_unchecked(alloc_size, layout.align()), + padded_size, + )) + } +} + +/// From +/// # Safety +/// The caller must ensure that: +/// - The resulting [`Layout`] is valid, by ensuring that `(layout.size() + padding_needed_for(layout, layout.align())) * n` doesn't overflow. +pub(super) unsafe fn array_layout_unchecked(layout: &Layout, n: usize) -> Layout { + let (array_layout, offset) = repeat_layout_unchecked(layout, n); + debug_assert_eq!(layout.size(), offset); + array_layout +} + +// TODO: replace with `Layout::repeat` if/when it stabilizes +/// From +/// # Safety +/// The caller must ensure that: +/// - The resulting [`Layout`] is valid, by ensuring that `(layout.size() + padding_needed_for(layout, layout.align())) * n` doesn't overflow. +unsafe fn repeat_layout_unchecked(layout: &Layout, n: usize) -> (Layout, usize) { + // This cannot overflow. Quoting from the invariant of Layout: + // > `size`, when rounded up to the nearest multiple of `align`, + // > must not overflow (i.e., the rounded value must be less than + // > `usize::MAX`) + let padded_size = layout.size() + padding_needed_for(layout, layout.align()); + // This may overflow in release builds, that's why this function is unsafe. + let alloc_size = padded_size * n; + + // SAFETY: self.align is already known to be valid and alloc_size has been + // padded already. + unsafe { + ( + Layout::from_size_align_unchecked(alloc_size, layout.align()), + padded_size, + ) + } +} + +/// From +const fn padding_needed_for(layout: &Layout, align: usize) -> usize { + let len = layout.size(); + + // Rounded up value is: + // len_rounded_up = (len + align - 1) & !(align - 1); + // and then we return the padding difference: `len_rounded_up - len`. + // + // We use modular arithmetic throughout: + // + // 1. align is guaranteed to be > 0, so align - 1 is always + // valid. + // + // 2. `len + align - 1` can overflow by at most `align - 1`, + // so the &-mask with `!(align - 1)` will ensure that in the + // case of overflow, `len_rounded_up` will itself be 0. + // Thus the returned padding, when added to `len`, yields 0, + // which trivially satisfies the alignment `align`. + // + // (Of course, attempts to allocate blocks of memory whose + // size and padding overflow in the above manner should cause + // the allocator to yield an error anyway.) + + let len_rounded_up = len.wrapping_add(align).wrapping_sub(1) & !align.wrapping_sub(1); + len_rounded_up.wrapping_sub(len) +} + #[cfg(test)] mod tests { use bevy_ecs::prelude::*; diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs deleted file mode 100644 index 85852a2bea81b..0000000000000 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ /dev/null @@ -1,724 +0,0 @@ -use alloc::alloc::handle_alloc_error; -use bevy_ptr::{OwningPtr, Ptr, PtrMut}; -use bevy_utils::OnDrop; -use core::{alloc::Layout, cell::UnsafeCell, num::NonZero, ptr::NonNull}; - -/// A flat, type-erased data storage type -/// -/// Used to densely store homogeneous ECS data. A blob is usually just an arbitrary block of contiguous memory without any identity, and -/// could be used to represent any arbitrary data (i.e. string, arrays, etc). This type is an extendable and re-allocatable blob, which makes -/// it a blobby Vec, a `BlobVec`. -pub(super) struct BlobVec { - item_layout: Layout, - capacity: usize, - /// Number of elements, not bytes - len: usize, - // the `data` ptr's layout is always `array_layout(item_layout, capacity)` - data: NonNull, - // None if the underlying type doesn't need to be dropped - drop: Option)>, -} - -// We want to ignore the `drop` field in our `Debug` impl -impl core::fmt::Debug for BlobVec { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - f.debug_struct("BlobVec") - .field("item_layout", &self.item_layout) - .field("capacity", &self.capacity) - .field("len", &self.len) - .field("data", &self.data) - .finish() - } -} - -impl BlobVec { - /// Creates a new [`BlobVec`] with the specified `capacity`. - /// - /// `drop` is an optional function pointer that is meant to be invoked when any element in the [`BlobVec`] - /// should be dropped. For all Rust-based types, this should match 1:1 with the implementation of [`Drop`] - /// if present, and should be `None` if `T: !Drop`. For non-Rust based types, this should match any cleanup - /// processes typically associated with the stored element. - /// - /// # Safety - /// - /// `drop` should be safe to call with an [`OwningPtr`] pointing to any item that's been pushed into this [`BlobVec`]. - /// - /// If `drop` is `None`, the items will be leaked. This should generally be set as None based on [`needs_drop`]. - /// - /// [`needs_drop`]: std::mem::needs_drop - pub unsafe fn new( - item_layout: Layout, - drop: Option)>, - capacity: usize, - ) -> BlobVec { - let align = NonZero::::new(item_layout.align()).expect("alignment must be > 0"); - let data = bevy_ptr::dangling_with_align(align); - if item_layout.size() == 0 { - BlobVec { - data, - // ZST `BlobVec` max size is `usize::MAX`, and `reserve_exact` for ZST assumes - // the capacity is always `usize::MAX` and panics if it overflows. - capacity: usize::MAX, - len: 0, - item_layout, - drop, - } - } else { - let mut blob_vec = BlobVec { - data, - capacity: 0, - len: 0, - item_layout, - drop, - }; - blob_vec.reserve_exact(capacity); - blob_vec - } - } - - /// Returns the number of elements in the vector. - #[inline] - pub fn len(&self) -> usize { - self.len - } - - /// Returns `true` if the vector contains no elements. - #[inline] - pub fn is_empty(&self) -> bool { - self.len == 0 - } - - /// Returns the [`Layout`] of the element type stored in the vector. - #[inline] - pub fn layout(&self) -> Layout { - self.item_layout - } - - /// Reserves the minimum capacity for at least `additional` more elements to be inserted in the given `BlobVec`. - /// After calling `reserve_exact`, capacity will be greater than or equal to `self.len() + additional`. Does nothing if - /// the capacity is already sufficient. - /// - /// Note that the allocator may give the collection more space than it requests. Therefore, capacity can not be relied upon - /// to be precisely minimal. - /// - /// # Panics - /// - /// Panics if new capacity overflows `usize`. - pub fn reserve_exact(&mut self, additional: usize) { - let available_space = self.capacity - self.len; - if available_space < additional { - // SAFETY: `available_space < additional`, so `additional - available_space > 0` - let increment = - unsafe { NonZero::::new_unchecked(additional - available_space) }; - self.grow_exact(increment); - } - } - - /// Reserves the minimum capacity for at least `additional` more elements to be inserted in the given `BlobVec`. - #[inline] - pub fn reserve(&mut self, additional: usize) { - /// Similar to `reserve_exact`. This method ensures that the capacity will grow at least `self.capacity()` if there is no - /// enough space to hold `additional` more elements. - #[cold] - fn do_reserve(slf: &mut BlobVec, additional: usize) { - let increment = slf.capacity.max(additional - (slf.capacity - slf.len)); - let increment = NonZero::::new(increment).unwrap(); - slf.grow_exact(increment); - } - - if self.capacity - self.len < additional { - do_reserve(self, additional); - } - } - - /// Grows the capacity by `increment` elements. - /// - /// # Panics - /// - /// Panics if the new capacity overflows `usize`. - /// For ZST it panics unconditionally because ZST `BlobVec` capacity - /// is initialized to `usize::MAX` and always stays that way. - fn grow_exact(&mut self, increment: NonZero) { - let new_capacity = self - .capacity - .checked_add(increment.get()) - .expect("capacity overflow"); - let new_layout = - array_layout(&self.item_layout, new_capacity).expect("array layout should be valid"); - let new_data = if self.capacity == 0 { - // SAFETY: - // - layout has non-zero size as per safety requirement - unsafe { alloc::alloc::alloc(new_layout) } - } else { - // SAFETY: - // - ptr was be allocated via this allocator - // - the layout of the ptr was `array_layout(self.item_layout, self.capacity)` - // - `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 overflow (i.e., the rounded value must be less than usize::MAX)", - // since the item size is always a multiple of its alignment, the rounding cannot happen - // here and the overflow is handled in `array_layout` - unsafe { - alloc::alloc::realloc( - self.get_ptr_mut().as_ptr(), - array_layout(&self.item_layout, self.capacity) - .expect("array layout should be valid"), - new_layout.size(), - ) - } - }; - - self.data = NonNull::new(new_data).unwrap_or_else(|| handle_alloc_error(new_layout)); - self.capacity = new_capacity; - } - - /// Initializes the value at `index` to `value`. This function does not do any bounds checking. - /// - /// # Safety - /// - index must be in bounds - /// - the memory in the [`BlobVec`] starting at index `index`, of a size matching this [`BlobVec`]'s - /// `item_layout`, must have been previously allocated. - #[inline] - pub unsafe fn initialize_unchecked(&mut self, index: usize, value: OwningPtr<'_>) { - debug_assert!(index < self.len()); - let ptr = self.get_unchecked_mut(index); - core::ptr::copy_nonoverlapping::(value.as_ptr(), ptr.as_ptr(), self.item_layout.size()); - } - - /// Replaces the value at `index` with `value`. This function does not do any bounds checking. - /// - /// # Safety - /// - index must be in-bounds - /// - the memory in the [`BlobVec`] starting at index `index`, of a size matching this - /// [`BlobVec`]'s `item_layout`, must have been previously initialized with an item matching - /// this [`BlobVec`]'s `item_layout` - /// - the memory at `*value` must also be previously initialized with an item matching this - /// [`BlobVec`]'s `item_layout` - pub unsafe fn replace_unchecked(&mut self, index: usize, value: OwningPtr<'_>) { - debug_assert!(index < self.len()); - - // Pointer to the value in the vector that will get replaced. - // SAFETY: The caller ensures that `index` fits in this vector. - let destination = NonNull::from(unsafe { self.get_unchecked_mut(index) }); - let source = value.as_ptr(); - - if let Some(drop) = self.drop { - // Temporarily set the length to zero, so that if `drop` panics the caller - // will not be left with a `BlobVec` containing a dropped element within - // its initialized range. - let old_len = self.len; - self.len = 0; - - // Transfer ownership of the old value out of the vector, so it can be dropped. - // SAFETY: - // - `destination` was obtained from a `PtrMut` in this vector, which ensures it is non-null, - // well-aligned for the underlying type, and has proper provenance. - // - The storage location will get overwritten with `value` later, which ensures - // that the element will not get observed or double dropped later. - // - If a panic occurs, `self.len` will remain `0`, which ensures a double-drop - // does not occur. Instead, all elements will be forgotten. - let old_value = unsafe { OwningPtr::new(destination) }; - - // This closure will run in case `drop()` panics, - // which ensures that `value` does not get forgotten. - let on_unwind = OnDrop::new(|| drop(value)); - - drop(old_value); - - // If the above code does not panic, make sure that `value` doesn't get dropped. - core::mem::forget(on_unwind); - - // Make the vector's contents observable again, since panics are no longer possible. - self.len = old_len; - } - - // Copy the new value into the vector, overwriting the previous value. - // SAFETY: - // - `source` and `destination` were obtained from `OwningPtr`s, which ensures they are - // valid for both reads and writes. - // - The value behind `source` will only be dropped if the above branch panics, - // so it must still be initialized and it is safe to transfer ownership into the vector. - // - `source` and `destination` were obtained from different memory locations, - // both of which we have exclusive access to, so they are guaranteed not to overlap. - unsafe { - core::ptr::copy_nonoverlapping::( - source, - destination.as_ptr(), - self.item_layout.size(), - ); - } - } - - /// Appends an element to the back of the vector. - /// - /// # Safety - /// The `value` must match the [`layout`](`BlobVec::layout`) of the elements in the [`BlobVec`]. - #[inline] - pub unsafe fn push(&mut self, value: OwningPtr<'_>) { - self.reserve(1); - let index = self.len; - self.len += 1; - self.initialize_unchecked(index, value); - } - - /// Performs a "swap remove" at the given `index`, which removes the item at `index` and moves - /// the last item in the [`BlobVec`] to `index` (if `index` is not the last item). It is the - /// caller's responsibility to drop the returned pointer, if that is desirable. - /// - /// # Safety - /// It is the caller's responsibility to ensure that `index` is less than `self.len()`. - #[must_use = "The returned pointer should be used to dropped the removed element"] - pub unsafe fn swap_remove_and_forget_unchecked(&mut self, index: usize) -> OwningPtr<'_> { - debug_assert!(index < self.len()); - // Since `index` must be strictly less than `self.len` and `index` is at least zero, - // `self.len` must be at least one. Thus, this cannot underflow. - let new_len = self.len - 1; - let size = self.item_layout.size(); - if index != new_len { - core::ptr::swap_nonoverlapping::( - self.get_unchecked_mut(index).as_ptr(), - self.get_unchecked_mut(new_len).as_ptr(), - size, - ); - } - self.len = new_len; - // Cannot use get_unchecked here as this is technically out of bounds after changing len. - // SAFETY: - // - `new_len` is less than the old len, so it must fit in this vector's allocation. - // - `size` is a multiple of the erased type's alignment, - // so adding a multiple of `size` will preserve alignment. - // - The removed element lives as long as this vector's mutable reference. - let p = unsafe { self.get_ptr_mut().byte_add(new_len * size) }; - // SAFETY: The removed element is unreachable by this vector so it's safe to promote the - // `PtrMut` to an `OwningPtr`. - unsafe { p.promote() } - } - - /// Removes the value at `index` and drops it. - /// Does not do any bounds checking on `index`. - /// The removed element is replaced by the last element of the `BlobVec`. - /// - /// # Safety - /// It is the caller's responsibility to ensure that `index` is `< self.len()`. - #[inline] - pub unsafe fn swap_remove_and_drop_unchecked(&mut self, index: usize) { - debug_assert!(index < self.len()); - let drop = self.drop; - let value = self.swap_remove_and_forget_unchecked(index); - if let Some(drop) = drop { - drop(value); - } - } - - /// Returns a reference to the element at `index`, without doing bounds checking. - /// - /// # Safety - /// It is the caller's responsibility to ensure that `index < self.len()`. - #[inline] - pub unsafe fn get_unchecked(&self, index: usize) -> Ptr<'_> { - debug_assert!(index < self.len()); - let size = self.item_layout.size(); - // SAFETY: - // - The caller ensures that `index` fits in this vector, - // so this operation will not overflow the original allocation. - // - `size` is a multiple of the erased type's alignment, - // so adding a multiple of `size` will preserve alignment. - // - The element at `index` outlives this vector's reference. - unsafe { self.get_ptr().byte_add(index * size) } - } - - /// Returns a mutable reference to the element at `index`, without doing bounds checking. - /// - /// # Safety - /// It is the caller's responsibility to ensure that `index < self.len()`. - #[inline] - pub unsafe fn get_unchecked_mut(&mut self, index: usize) -> PtrMut<'_> { - debug_assert!(index < self.len()); - let size = self.item_layout.size(); - // SAFETY: - // - The caller ensures that `index` fits in this vector, - // so this operation will not overflow the original allocation. - // - `size` is a multiple of the erased type's alignment, - // so adding a multiple of `size` will preserve alignment. - // - The element at `index` outlives this vector's mutable reference. - unsafe { self.get_ptr_mut().byte_add(index * size) } - } - - /// Gets a [`Ptr`] to the start of the vec - #[inline] - pub fn get_ptr(&self) -> Ptr<'_> { - // SAFETY: the inner data will remain valid for as long as 'self. - unsafe { Ptr::new(self.data) } - } - - /// Gets a [`PtrMut`] to the start of the vec - #[inline] - pub fn get_ptr_mut(&mut self) -> PtrMut<'_> { - // SAFETY: the inner data will remain valid for as long as 'self. - unsafe { PtrMut::new(self.data) } - } - - /// Get a reference to the entire [`BlobVec`] as if it were an array with elements of type `T` - /// - /// # Safety - /// The type `T` must be the type of the items in this [`BlobVec`]. - pub unsafe fn get_slice(&self) -> &[UnsafeCell] { - // SAFETY: the inner data will remain valid for as long as 'self. - unsafe { core::slice::from_raw_parts(self.data.as_ptr() as *const UnsafeCell, self.len) } - } - - /// Returns the drop function for values stored in the vector, - /// or `None` if they don't need to be dropped. - #[inline] - pub fn get_drop(&self) -> Option)> { - self.drop - } - - /// Clears the vector, removing (and dropping) all values. - /// - /// Note that this method has no effect on the allocated capacity of the vector. - pub fn clear(&mut self) { - let len = self.len; - // We set len to 0 _before_ dropping elements for unwind safety. This ensures we don't - // accidentally drop elements twice in the event of a drop impl panicking. - self.len = 0; - if let Some(drop) = self.drop { - let size = self.item_layout.size(); - for i in 0..len { - // SAFETY: - // * 0 <= `i` < `len`, so `i * size` must be in bounds for the allocation. - // * `size` is a multiple of the erased type's alignment, - // so adding a multiple of `size` will preserve alignment. - // * The item lives until it's dropped. - // * The item is left unreachable so it can be safely promoted to an `OwningPtr`. - // NOTE: `self.get_unchecked_mut(i)` cannot be used here, since the `debug_assert` - // would panic due to `self.len` being set to 0. - let item = unsafe { self.get_ptr_mut().byte_add(i * size).promote() }; - // SAFETY: `item` was obtained from this `BlobVec`, so its underlying type must match `drop`. - unsafe { drop(item) }; - } - } - } -} - -impl Drop for BlobVec { - fn drop(&mut self) { - self.clear(); - let array_layout = - array_layout(&self.item_layout, self.capacity).expect("array layout should be valid"); - if array_layout.size() > 0 { - // SAFETY: data ptr layout is correct, swap_scratch ptr layout is correct - unsafe { - alloc::alloc::dealloc(self.get_ptr_mut().as_ptr(), array_layout); - } - } - } -} - -/// From -pub(super) fn array_layout(layout: &Layout, n: usize) -> Option { - let (array_layout, offset) = repeat_layout(layout, n)?; - debug_assert_eq!(layout.size(), offset); - Some(array_layout) -} - -// TODO: replace with `Layout::repeat` if/when it stabilizes -/// From -fn repeat_layout(layout: &Layout, n: usize) -> Option<(Layout, usize)> { - // This cannot overflow. Quoting from the invariant of Layout: - // > `size`, when rounded up to the nearest multiple of `align`, - // > must not overflow (i.e., the rounded value must be less than - // > `usize::MAX`) - let padded_size = layout.size() + padding_needed_for(layout, layout.align()); - let alloc_size = padded_size.checked_mul(n)?; - - // SAFETY: self.align is already known to be valid and alloc_size has been - // padded already. - unsafe { - Some(( - Layout::from_size_align_unchecked(alloc_size, layout.align()), - padded_size, - )) - } -} - -/// From -/// # Safety -/// The caller must ensure that: -/// - The resulting [`Layout`] is valid, by ensuring that `(layout.size() + padding_needed_for(layout, layout.align())) * n` doesn't overflow. -pub(super) unsafe fn array_layout_unchecked(layout: &Layout, n: usize) -> Layout { - let (array_layout, offset) = repeat_layout_unchecked(layout, n); - debug_assert_eq!(layout.size(), offset); - array_layout -} - -// TODO: replace with `Layout::repeat` if/when it stabilizes -/// From -/// # Safety -/// The caller must ensure that: -/// - The resulting [`Layout`] is valid, by ensuring that `(layout.size() + padding_needed_for(layout, layout.align())) * n` doesn't overflow. -unsafe fn repeat_layout_unchecked(layout: &Layout, n: usize) -> (Layout, usize) { - // This cannot overflow. Quoting from the invariant of Layout: - // > `size`, when rounded up to the nearest multiple of `align`, - // > must not overflow (i.e., the rounded value must be less than - // > `usize::MAX`) - let padded_size = layout.size() + padding_needed_for(layout, layout.align()); - // This may overflow in release builds, that's why this function is unsafe. - let alloc_size = padded_size * n; - - // SAFETY: self.align is already known to be valid and alloc_size has been - // padded already. - unsafe { - ( - Layout::from_size_align_unchecked(alloc_size, layout.align()), - padded_size, - ) - } -} - -/// From -const fn padding_needed_for(layout: &Layout, align: usize) -> usize { - let len = layout.size(); - - // Rounded up value is: - // len_rounded_up = (len + align - 1) & !(align - 1); - // and then we return the padding difference: `len_rounded_up - len`. - // - // We use modular arithmetic throughout: - // - // 1. align is guaranteed to be > 0, so align - 1 is always - // valid. - // - // 2. `len + align - 1` can overflow by at most `align - 1`, - // so the &-mask with `!(align - 1)` will ensure that in the - // case of overflow, `len_rounded_up` will itself be 0. - // Thus the returned padding, when added to `len`, yields 0, - // which trivially satisfies the alignment `align`. - // - // (Of course, attempts to allocate blocks of memory whose - // size and padding overflow in the above manner should cause - // the allocator to yield an error anyway.) - - let len_rounded_up = len.wrapping_add(align).wrapping_sub(1) & !align.wrapping_sub(1); - len_rounded_up.wrapping_sub(len) -} - -#[cfg(test)] -mod tests { - use super::BlobVec; - use crate::{component::Component, ptr::OwningPtr, world::World}; - use alloc::{ - rc::Rc, - string::{String, ToString}, - }; - use core::{alloc::Layout, cell::RefCell}; - - /// # Safety - /// - /// The pointer `x` must point to a valid value of type `T` and it must be safe to drop this value. - unsafe fn drop_ptr(x: OwningPtr<'_>) { - // SAFETY: It is guaranteed by the caller that `x` points to a - // valid value of type `T` and it is safe to drop this value. - unsafe { - x.drop_as::(); - } - } - - /// # Safety - /// - /// `blob_vec` must have a layout that matches `Layout::new::()` - unsafe fn push(blob_vec: &mut BlobVec, value: T) { - OwningPtr::make(value, |ptr| { - blob_vec.push(ptr); - }); - } - - /// # Safety - /// - /// `blob_vec` must have a layout that matches `Layout::new::()` - unsafe fn swap_remove(blob_vec: &mut BlobVec, index: usize) -> T { - assert!(index < blob_vec.len()); - let value = blob_vec.swap_remove_and_forget_unchecked(index); - value.read::() - } - - /// # Safety - /// - /// `blob_vec` must have a layout that matches `Layout::new::()`, it most store a valid `T` - /// value at the given `index` - unsafe fn get_mut(blob_vec: &mut BlobVec, index: usize) -> &mut T { - assert!(index < blob_vec.len()); - blob_vec.get_unchecked_mut(index).deref_mut::() - } - - #[test] - fn resize_test() { - let item_layout = Layout::new::(); - // SAFETY: `drop` fn is `None`, usize doesn't need dropping - let mut blob_vec = unsafe { BlobVec::new(item_layout, None, 64) }; - // SAFETY: `i` is a usize, i.e. the type corresponding to `item_layout` - unsafe { - for i in 0..1_000 { - push(&mut blob_vec, i as usize); - } - } - - assert_eq!(blob_vec.len(), 1_000); - assert_eq!(blob_vec.capacity, 1_024); - } - - #[derive(Debug, Eq, PartialEq, Clone)] - struct Foo { - a: u8, - b: String, - drop_counter: Rc>, - } - - impl Drop for Foo { - fn drop(&mut self) { - *self.drop_counter.borrow_mut() += 1; - } - } - - #[test] - fn blob_vec() { - let drop_counter = Rc::new(RefCell::new(0)); - { - let item_layout = Layout::new::(); - let drop = drop_ptr::; - // SAFETY: drop is able to drop a value of its `item_layout` - let mut blob_vec = unsafe { BlobVec::new(item_layout, Some(drop), 2) }; - assert_eq!(blob_vec.capacity, 2); - // SAFETY: the following code only deals with values of type `Foo`, which satisfies the safety requirement of `push`, `get_mut` and `swap_remove` that the - // values have a layout compatible to the blob vec's `item_layout`. - // Every index is in range. - unsafe { - let foo1 = Foo { - a: 42, - b: "abc".to_string(), - drop_counter: drop_counter.clone(), - }; - push(&mut blob_vec, foo1.clone()); - assert_eq!(blob_vec.len(), 1); - assert_eq!(get_mut::(&mut blob_vec, 0), &foo1); - - let mut foo2 = Foo { - a: 7, - b: "xyz".to_string(), - drop_counter: drop_counter.clone(), - }; - push::(&mut blob_vec, foo2.clone()); - assert_eq!(blob_vec.len(), 2); - assert_eq!(blob_vec.capacity, 2); - assert_eq!(get_mut::(&mut blob_vec, 0), &foo1); - assert_eq!(get_mut::(&mut blob_vec, 1), &foo2); - - get_mut::(&mut blob_vec, 1).a += 1; - assert_eq!(get_mut::(&mut blob_vec, 1).a, 8); - - let foo3 = Foo { - a: 16, - b: "123".to_string(), - drop_counter: drop_counter.clone(), - }; - - push(&mut blob_vec, foo3.clone()); - assert_eq!(blob_vec.len(), 3); - assert_eq!(blob_vec.capacity, 4); - - let last_index = blob_vec.len() - 1; - let value = swap_remove::(&mut blob_vec, last_index); - assert_eq!(foo3, value); - - assert_eq!(blob_vec.len(), 2); - assert_eq!(blob_vec.capacity, 4); - - let value = swap_remove::(&mut blob_vec, 0); - assert_eq!(foo1, value); - assert_eq!(blob_vec.len(), 1); - assert_eq!(blob_vec.capacity, 4); - - foo2.a = 8; - assert_eq!(get_mut::(&mut blob_vec, 0), &foo2); - } - } - - assert_eq!(*drop_counter.borrow(), 6); - } - - #[test] - fn blob_vec_drop_empty_capacity() { - let item_layout = Layout::new::(); - let drop = drop_ptr::; - // SAFETY: drop is able to drop a value of its `item_layout` - let _ = unsafe { BlobVec::new(item_layout, Some(drop), 0) }; - } - - #[test] - #[should_panic(expected = "capacity overflow")] - fn blob_vec_zst_size_overflow() { - // SAFETY: no drop is correct drop for `()`. - let mut blob_vec = unsafe { BlobVec::new(Layout::new::<()>(), None, 0) }; - - assert_eq!(usize::MAX, blob_vec.capacity, "Self-check"); - - // SAFETY: Because `()` is a ZST trivial drop type, and because `BlobVec` capacity - // is always `usize::MAX` for ZSTs, we can arbitrarily set the length - // and still be sound. - blob_vec.len = usize::MAX; - - // SAFETY: `BlobVec` was initialized for `()`, so it is safe to push `()` to it. - unsafe { - OwningPtr::make((), |ptr| { - // This should panic because len is usize::MAX, remaining capacity is 0. - blob_vec.push(ptr); - }); - } - } - - #[test] - #[should_panic(expected = "capacity overflow")] - fn blob_vec_capacity_overflow() { - // SAFETY: no drop is correct drop for `u32`. - let mut blob_vec = unsafe { BlobVec::new(Layout::new::(), None, 0) }; - - assert_eq!(0, blob_vec.capacity, "Self-check"); - - OwningPtr::make(17u32, |ptr| { - // SAFETY: we push the value of correct type. - unsafe { - blob_vec.push(ptr); - } - }); - - blob_vec.reserve_exact(usize::MAX); - } - - #[test] - fn aligned_zst() { - // NOTE: This test is explicitly for uncovering potential UB with miri. - - #[derive(Component)] - #[repr(align(32))] - struct Zst; - - let mut world = World::default(); - world.spawn(Zst); - world.spawn(Zst); - world.spawn(Zst); - world.spawn_empty(); - - let mut count = 0; - - let mut q = world.query::<&Zst>(); - for zst in q.iter(&world) { - // Ensure that the references returned are properly aligned. - assert_eq!( - core::ptr::from_ref::(zst) as usize % align_of::(), - 0 - ); - count += 1; - } - - assert_eq!(count, 3); - } -} diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index 2a5a5f184e649..852be11cad13e 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -21,7 +21,6 @@ //! [`World::storages`]: crate::world::World::storages mod blob_array; -mod blob_vec; mod resource; mod sparse_set; mod table; @@ -32,6 +31,7 @@ pub use sparse_set::*; pub use table::*; use crate::component::{ComponentInfo, StorageType}; +use alloc::vec::Vec; /// The raw data stores of a [`World`](crate::world::World) #[derive(Default)] @@ -60,3 +60,50 @@ impl Storages { } } } + +struct AbortOnPanic; + +impl Drop for AbortOnPanic { + fn drop(&mut self) { + // Panicking while unwinding will force an abort. + panic!("Aborting due to allocator error"); + } +} + +/// Unsafe extension functions for `Vec` +trait VecExtensions { + /// Removes an element from the vector and returns it. + /// + /// The removed element is replaced by the last element of the vector. + /// + /// This does not preserve ordering of the remaining elements, but is O(1). If you need to preserve the element order, use [`remove`] instead. + /// + /// Unlike [`swap_remove`], this does not panic if `index` is out of bounds. + /// + /// # Safety + /// + /// All of the following must be true: + /// - `self.len() > 1` + /// - `index < self.len() - 1` + /// + /// [`remove`]: alloc::vec::Vec::remove + /// [`swap_remove`]: alloc::vec::Vec::swap_remove + unsafe fn swap_remove_nonoverlapping_unchecked(&mut self, index: usize) -> T; +} + +impl VecExtensions for Vec { + #[inline] + unsafe fn swap_remove_nonoverlapping_unchecked(&mut self, index: usize) -> T { + // SAFETY: The caller must ensure that the element at `index` must be valid. + // This function, and then the caller takes ownership of the value, and it cannot be + // accessed due to the length being decremented immediately after this. + let value = unsafe { self.as_mut_ptr().add(index).read() }; + let len = self.len(); + let base_ptr = self.as_mut_ptr(); + // SAFETY: We replace self[index] with the last element. The caller must ensure that + // both the last element and `index` must be valid and cannot point to the same place. + unsafe { core::ptr::copy_nonoverlapping(base_ptr.add(len - 1), base_ptr.add(index), 1) }; + self.set_len(len - 1); + value + } +} diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 1a90cc511ccc0..e84b5b520a209 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -1,11 +1,11 @@ use crate::{ change_detection::{MaybeLocation, MutUntyped, TicksMut}, component::{CheckChangeTicks, ComponentId, ComponentTicks, Components, Tick, TickCells}, - storage::{blob_vec::BlobVec, SparseSet}, + storage::{blob_array::BlobArray, SparseSet}, }; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; use bevy_utils::prelude::DebugName; -use core::{cell::UnsafeCell, mem::ManuallyDrop, panic::Location}; +use core::{cell::UnsafeCell, panic::Location}; #[cfg(feature = "std")] use std::thread::ThreadId; @@ -16,7 +16,8 @@ use std::thread::ThreadId; /// /// [`World`]: crate::world::World pub struct ResourceData { - data: ManuallyDrop, + data: BlobArray, + is_present: bool, added_ticks: UnsafeCell, changed_ticks: UnsafeCell, #[cfg_attr( @@ -49,13 +50,13 @@ impl Drop for ResourceData { // 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.data); + self.data.drop(1, self.is_present().into()); } } } impl ResourceData { - /// The only row in the underlying `BlobVec`. + /// The only row in the underlying `BlobArray`. const ROW: usize = 0; /// Validates the access to `!Send` resources is only done on the thread they were created from. @@ -86,7 +87,7 @@ impl ResourceData { /// Returns true if the resource is populated. #[inline] pub fn is_present(&self) -> bool { - !self.data.is_empty() + self.is_present } /// Returns a reference to the resource, if it exists. @@ -181,16 +182,20 @@ impl ResourceData { // 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. - unsafe { - self.data.replace_unchecked(Self::ROW, value); - } + unsafe { self.data.replace_unchecked(Self::ROW, value) }; } else { #[cfg(feature = "std")] if !SEND { self.origin_thread_id = Some(std::thread::current().id()); } - self.data.push(value); + // SAFETY: + // - There is only one element, and it's always allocated. + // - The caller guarantees must be valid for the underlying type and thus its + // layout must be identical. + // - The value was previously not present and thus must not have been initialized. + unsafe { self.data.initialize_unchecked(Self::ROW, value) }; *self.added_ticks.deref_mut() = change_tick; + self.is_present = true; } *self.changed_ticks.deref_mut() = change_tick; @@ -221,15 +226,19 @@ impl ResourceData { // 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. - unsafe { - self.data.replace_unchecked(Self::ROW, value); - } + unsafe { self.data.replace_unchecked(Self::ROW, value) }; } else { #[cfg(feature = "std")] if !SEND { self.origin_thread_id = Some(std::thread::current().id()); } - self.data.push(value); + // SAFETY: + // - There is only one element, and it's always allocated. + // - The caller guarantees must be valid for the underlying type and thus its + // layout must be identical. + // - The value was previously not present and thus must not have been initialized. + unsafe { self.data.initialize_unchecked(Self::ROW, value) }; + self.is_present = true; } *self.added_ticks.deref_mut() = change_ticks.added; *self.changed_ticks.deref_mut() = change_ticks.changed; @@ -253,8 +262,14 @@ impl ResourceData { 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) }; + + self.is_present = false; + + // SAFETY: + // - There is always only one row in the `BlobArray` created during initialization. + // - This function has validated that the row is present with the check of `self.is_present`. + // - The caller is to take ownership of the value, returned as a `OwningPtr`. + let res = unsafe { self.data.get_unchecked_mut(Self::ROW).promote() }; let caller = self .changed_by @@ -285,7 +300,9 @@ impl ResourceData { pub(crate) fn remove_and_drop(&mut self) { if self.is_present() { self.validate_access(); - self.data.clear(); + // SAFETY: There is only one element, and it's always allocated. + unsafe { self.data.drop_last_element(Self::ROW) }; + self.is_present = false; } } @@ -366,14 +383,15 @@ impl Resources { } // SAFETY: component_info.drop() is valid for the types that will be inserted. let data = unsafe { - BlobVec::new( + BlobArray::with_capacity( component_info.layout(), component_info.drop(), 1 ) }; ResourceData { - data: ManuallyDrop::new(data), + data, + is_present: false, added_ticks: UnsafeCell::new(Tick::new(0)), changed_ticks: UnsafeCell::new(Tick::new(0)), type_name: component_info.name(), diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index bb28f967af377..3cf40a578494f 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -2,11 +2,12 @@ use crate::{ change_detection::MaybeLocation, component::{CheckChangeTicks, ComponentId, ComponentInfo, ComponentTicks, Tick, TickCells}, entity::{Entity, EntityRow}, - storage::{Column, TableRow}, + query::DebugCheckedUnwrap, + storage::{AbortOnPanic, TableRow, ThinColumn, VecExtensions}, }; use alloc::{boxed::Box, vec::Vec}; use bevy_ptr::{OwningPtr, Ptr}; -use core::{cell::UnsafeCell, hash::Hash, marker::PhantomData, panic::Location}; +use core::{cell::UnsafeCell, hash::Hash, marker::PhantomData, num::NonZero, panic::Location}; use nonmax::{NonMaxU32, NonMaxUsize}; #[derive(Debug)] @@ -67,6 +68,10 @@ impl_sparse_array!(ImmutableSparseArray); impl SparseArray { /// Inserts `value` at `index` in the array. /// + /// # Panics + /// - Panics if the insertion forces a reallocation, and any of the new capacity overflows `isize::MAX` bytes. + /// - Panics if the insertion forces a reallocation, and any of the new the reallocations causes an out-of-memory error. + /// /// If `index` is out-of-bounds, this will enlarge the buffer to accommodate it. #[inline] pub fn insert(&mut self, index: I, value: V) { @@ -114,7 +119,7 @@ impl SparseArray { /// Designed for relatively fast insertions and deletions. #[derive(Debug)] pub struct ComponentSparseSet { - dense: Column, + dense: ThinColumn, // Internally this only relies on the Entity index to keep track of where the component data is // stored for entities that are alive. The generation is not required, but is stored // in debug builds to validate that access is correct. @@ -129,16 +134,18 @@ impl ComponentSparseSet { /// Creates a new [`ComponentSparseSet`] with a given component type layout and /// initial `capacity`. pub(crate) fn new(component_info: &ComponentInfo, capacity: usize) -> Self { + let entities = Vec::with_capacity(capacity); Self { - dense: Column::with_capacity(component_info, capacity), - entities: Vec::with_capacity(capacity), + dense: ThinColumn::with_capacity(component_info, entities.capacity()), + entities, sparse: Default::default(), } } /// Removes all of the values stored within. pub(crate) fn clear(&mut self) { - self.dense.clear(); + // SAFETY: This is using the size of the ComponentSparseSet. + unsafe { self.dense.clear(self.len()) }; self.entities.clear(); self.sparse.clear(); } @@ -146,18 +153,22 @@ impl ComponentSparseSet { /// Returns the number of component values in the sparse set. #[inline] pub fn len(&self) -> usize { - self.dense.len() + self.entities.len() } /// Returns `true` if the sparse set contains no component values. #[inline] pub fn is_empty(&self) -> bool { - self.dense.len() == 0 + self.entities.is_empty() } /// Inserts the `entity` key and component `value` pair into this sparse /// set. /// + /// # Aborts + /// - Aborts the process if the insertion forces a reallocation, and any of the new capacity overflows `isize::MAX` bytes. + /// - Aborts the process if the insertion forces a reallocation, and any of the new the reallocations causes an out-of-memory error. + /// /// # Safety /// The `value` pointer must point to a valid address that matches the [`Layout`](std::alloc::Layout) /// inside the [`ComponentInfo`] given when constructing this sparse set. @@ -173,22 +184,37 @@ impl ComponentSparseSet { assert_eq!(entity, self.entities[dense_index.index()]); self.dense.replace(dense_index, value, change_tick, caller); } else { - let dense_index = self.dense.len(); - self.dense - .push(value, ComponentTicks::new(change_tick), caller); + let dense_index = self.entities.len(); + let capacity = self.entities.capacity(); + + #[cfg(not(debug_assertions))] + self.entities.push(entity.row()); + #[cfg(debug_assertions)] + self.entities.push(entity); + + // If any of the following operations panic due to an allocation error, the state + // of the `ComponentSparseSet` will be left in an invalid state and potentially cause UB. + // We create an AbortOnPanic guard to force panics to terminate the process if this occurs. + let _guard = AbortOnPanic; + if capacity != self.entities.capacity() { + // SAFETY: An entity was just pushed onto `entities`, its capacity cannot be zero. + let new_capacity = unsafe { NonZero::new_unchecked(self.entities.capacity()) }; + if let Some(capacity) = NonZero::new(capacity) { + // SAFETY: This is using the layout of the previous allocation. + unsafe { self.dense.realloc(capacity, new_capacity) }; + } else { + self.dense.alloc(new_capacity); + } + } // SAFETY: This entity row does not exist here yet, so there are no duplicates, // and the entity index can not be the max, so the length must not be max either. // To do so would have caused a panic in the entity alloxator. let table_row = unsafe { TableRow::new(NonMaxU32::new_unchecked(dense_index as u32)) }; - + self.dense.initialize(table_row, value, change_tick, caller); self.sparse.insert(entity.row(), table_row); - #[cfg(debug_assertions)] - assert_eq!(self.entities.len(), dense_index); - #[cfg(not(debug_assertions))] - self.entities.push(entity.row()); - #[cfg(debug_assertions)] - self.entities.push(entity); + + core::mem::forget(_guard); } } @@ -317,19 +343,46 @@ impl ComponentSparseSet { self.sparse.remove(entity.row()).map(|dense_index| { #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.index()]); - self.entities.swap_remove(dense_index.index()); - let is_last = dense_index.index() == self.dense.len() - 1; - // SAFETY: dense_index was just removed from `sparse`, which ensures that it is valid - let (value, _, _) = unsafe { self.dense.swap_remove_and_forget_unchecked(dense_index) }; - if !is_last { - let swapped_entity = self.entities[dense_index.index()]; + let last = self.entities.len() - 1; + if dense_index.index() >= last { + #[cfg(debug_assertions)] + assert_eq!(dense_index.index(), last); + // SAFETY: This is strictly decreasing the length, so it cannot outgrow + // it also cannot underflow as an item was just removed from the sparse array. + unsafe { self.entities.set_len(last) }; + // SAFETY: `last` is guaranteed to be the last element in `dense` as the length is synced with + // the `entities` store. + unsafe { + self.dense + .get_data_unchecked(dense_index) + .assert_unique() + .promote() + } + } else { + // SAFETY: The above check ensures that `dense_index` and the last element are not + // overlapping, and thus also within bounds. + unsafe { + self.entities + .swap_remove_nonoverlapping_unchecked(dense_index.index()); + }; + // SAFETY: The above check ensures that `dense_index` is in bounds. + let swapped_entity = unsafe { self.entities.get_unchecked(dense_index.index()) }; #[cfg(not(debug_assertions))] - let index = swapped_entity; + let index = *swapped_entity; #[cfg(debug_assertions)] let index = swapped_entity.row(); - *self.sparse.get_mut(index).unwrap() = dense_index; + // SAFETY: The swapped entity was just fetched from the entity Vec, it must have already + // been inserted and in bounds. + unsafe { + *self.sparse.get_mut(index).debug_checked_unwrap() = dense_index; + } + // SAFETY: The above check ensures that `dense_index` and the last element are not + // overlapping, and thus also within bounds. + unsafe { + self.dense + .swap_remove_and_forget_unchecked_nonoverlapping(last, dense_index) + } } - value }) } @@ -337,31 +390,65 @@ impl ComponentSparseSet { /// /// Returns `true` if `entity` had a component value in the sparse set. pub(crate) fn remove(&mut self, entity: Entity) -> bool { - if let Some(dense_index) = self.sparse.remove(entity.row()) { - #[cfg(debug_assertions)] - assert_eq!(entity, self.entities[dense_index.index()]); - self.entities.swap_remove(dense_index.index()); - let is_last = dense_index.index() == self.dense.len() - 1; - // SAFETY: if the sparse index points to something in the dense vec, it exists - unsafe { - self.dense.swap_remove_unchecked(dense_index); - } - if !is_last { - let swapped_entity = self.entities[dense_index.index()]; - #[cfg(not(debug_assertions))] - let index = swapped_entity; + self.sparse + .remove(entity.row()) + .map(|dense_index| { #[cfg(debug_assertions)] - let index = swapped_entity.row(); - *self.sparse.get_mut(index).unwrap() = dense_index; - } - true - } else { - false - } + assert_eq!(entity, self.entities[dense_index.index()]); + let last = self.entities.len() - 1; + if dense_index.index() >= last { + #[cfg(debug_assertions)] + assert_eq!(dense_index.index(), last); + // SAFETY: This is strictly decreasing the length, so it cannot outgrow + // it also cannot underflow as an item was just removed from the sparse array. + unsafe { self.entities.set_len(last) }; + // SAFETY: `last` is guaranteed to be the last element in `dense` as the length is synced with + // the `entities` store. + unsafe { self.dense.drop_last_component(last) }; + } else { + // SAFETY: The above check ensures that `dense_index` and the last element are not + // overlapping, and thus also within bounds. + unsafe { + self.entities + .swap_remove_nonoverlapping_unchecked(dense_index.index()); + }; + let swapped_entity = + // SAFETY: The above check ensures that `dense_index` is in bounds. + unsafe { self.entities.get_unchecked(dense_index.index()) }; + #[cfg(not(debug_assertions))] + let index = *swapped_entity; + #[cfg(debug_assertions)] + let index = swapped_entity.row(); + // SAFETY: The swapped entity was just fetched from the entity Vec, it must have already + // been inserted and in bounds. + unsafe { + *self.sparse.get_mut(index).debug_checked_unwrap() = dense_index; + } + // SAFETY: The above check ensures that `dense_index` and the last element are not + // overlapping, and thus also within bounds. + unsafe { + self.dense + .swap_remove_and_drop_unchecked_nonoverlapping(last, dense_index); + } + } + }) + .is_some() } pub(crate) fn check_change_ticks(&mut self, check: CheckChangeTicks) { - self.dense.check_change_ticks(check); + // SAFETY: This is using the valid size of the column. + unsafe { self.dense.check_change_ticks(self.len(), check) }; + } +} + +impl Drop for ComponentSparseSet { + fn drop(&mut self) { + let len = self.entities.len(); + self.entities.clear(); + // SAFETY: `cap` and `len` are correct. `dense` is never accessed again after this call. + unsafe { + self.dense.drop(self.entities.capacity(), len); + } } } @@ -470,6 +557,10 @@ impl SparseSet { impl SparseSet { /// Creates a new [`SparseSet`] with a specified initial capacity. + /// + /// # Panics + /// - Panics if the new capacity of the allocation overflows `isize::MAX` bytes. + /// - Panics if the new allocation causes an out-of-memory error. pub fn with_capacity(capacity: usize) -> Self { Self { dense: Vec::with_capacity(capacity), @@ -487,6 +578,10 @@ impl SparseSet { /// Inserts `value` at `index`. /// /// If a value was already present at `index`, it will be overwritten. + /// + /// # Panics + /// - Panics if the insertion forces an reallocation and the new capacity overflows `isize::MAX` bytes. + /// - Panics if the insertion forces an reallocation and causes an out-of-memory error. pub fn insert(&mut self, index: I, value: V) { if let Some(dense_index) = self.sparse.get(index.clone()).cloned() { // SAFETY: dense indices stored in self.sparse always exist @@ -503,6 +598,10 @@ impl SparseSet { /// Returns a reference to the value for `index`, inserting one computed from `func` /// if not already present. + /// + /// # Panics + /// - Panics if the insertion forces an reallocation and the new capacity overflows `isize::MAX` bytes. + /// - Panics if the insertion forces an reallocation and causes an out-of-memory error. pub fn get_or_insert_with(&mut self, index: I, func: impl FnOnce() -> V) -> &mut V { if let Some(dense_index) = self.sparse.get(index.clone()).cloned() { // SAFETY: dense indices stored in self.sparse always exist @@ -543,6 +642,9 @@ impl SparseSet { } /// Clears all of the elements from the sparse set. + /// + /// # Panics + /// - Panics if any of the keys or values implements [`Drop`] and any of those panic. pub fn clear(&mut self) { self.dense.clear(); self.indices.clear(); @@ -624,6 +726,10 @@ impl SparseSets { /// Gets a mutable reference of [`ComponentSparseSet`] of a [`ComponentInfo`]. /// Create a new [`ComponentSparseSet`] if not exists. + /// + /// # Panics + /// - Panics if the insertion forces an reallocation and the new capacity overflows `isize::MAX` bytes. + /// - Panics if the insertion forces an reallocation and causes an out-of-memory error. pub(crate) fn get_or_insert( &mut self, component_info: &ComponentInfo, @@ -644,6 +750,9 @@ impl SparseSets { } /// Clear entities stored in each [`ComponentSparseSet`] + /// + /// # Panics + /// - Panics if any of the components stored within implement [`Drop`] and any of them panic. pub(crate) fn clear_entities(&mut self) { for set in self.sets.values_mut() { set.clear(); diff --git a/crates/bevy_ecs/src/storage/table/column.rs b/crates/bevy_ecs/src/storage/table/column.rs index acf531d9b9d1c..d21777c6704e6 100644 --- a/crates/bevy_ecs/src/storage/table/column.rs +++ b/crates/bevy_ecs/src/storage/table/column.rs @@ -1,20 +1,27 @@ use super::*; use crate::{ change_detection::MaybeLocation, - component::TickCells, storage::{blob_array::BlobArray, thin_array_ptr::ThinArrayPtr}, }; -use alloc::vec::Vec; -use bevy_ptr::PtrMut; use core::panic::Location; -/// Very similar to a normal [`Column`], but with the capacities and lengths cut out for performance reasons. +/// A type-erased contiguous container for data of a homogeneous type. /// -/// This type is used by [`Table`], because all of the capacities and lengths of the [`Table`]'s columns must match. +/// Conceptually, a `ThinColumn` is very similar to a type-erased `Box<[T]>`. +/// It also stores the change detection ticks for its components, kept in two separate +/// contiguous buffers internally. An element shares its data across these buffers by using the +/// same index (i.e. the entity at row 3 has it's data at index 3 and its change detection ticks at index 3). /// -/// Like many other low-level storage types, [`ThinColumn`] has a limited and highly unsafe +/// Like many other low-level storage types, `ThinColumn` has a limited and highly unsafe /// interface. It's highly advised to use higher level types and their safe abstractions -/// instead of working directly with [`ThinColumn`]. +/// instead of working directly with `ThinColumn`. +/// +/// For performance reasons, `ThinColumn` does not does not store it's capacity and length. +/// This type is used by [`Table`] and [`ComponentSparseSet`], because all of the capacities +/// and lengths of the owning storaage. +/// +/// [`ComponentSparseSet`]: crate::storage::ComponentSparseSet +#[derive(Debug)] pub struct ThinColumn { pub(super) data: BlobArray, pub(super) added_ticks: ThinArrayPtr>, @@ -81,6 +88,31 @@ impl ThinColumn { }); } + /// Swap-remove and forgets the removed element, but the component at `row` must not be the last element. + /// + /// # Safety + /// - `row.as_usize()` < `len` + /// - `last_element_index` = `len - 1` + /// - `last_element_index` != `row.as_usize()` + /// - The caller should update the `len` to `len - 1`, or immediately initialize another element in the `last_element_index` + pub(crate) unsafe fn swap_remove_and_forget_unchecked_nonoverlapping( + &mut self, + last_element_index: usize, + row: TableRow, + ) -> OwningPtr<'_> { + let data = self + .data + .swap_remove_unchecked_nonoverlapping(row.index(), last_element_index); + self.added_ticks + .swap_remove_unchecked_nonoverlapping(row.index(), last_element_index); + self.changed_ticks + .swap_remove_unchecked_nonoverlapping(row.index(), last_element_index); + self.changed_by.as_mut().map(|changed_by| { + changed_by.swap_remove_unchecked_nonoverlapping(row.index(), last_element_index); + }); + data + } + /// Swap-remove and forget the removed element. /// /// # Safety @@ -91,8 +123,8 @@ impl ThinColumn { &mut self, last_element_index: usize, row: TableRow, - ) { - let _ = self + ) -> OwningPtr<'_> { + let data = self .data .swap_remove_unchecked(row.index(), last_element_index); self.added_ticks @@ -102,10 +134,15 @@ impl ThinColumn { self.changed_by .as_mut() .map(|changed_by| changed_by.swap_remove_unchecked(row.index(), last_element_index)); + data } /// Call [`realloc`](std::alloc::realloc) to expand / shrink the memory allocation for this [`ThinColumn`] /// + /// # Panics + /// - Panics if the any of the new capacity overflows `isize::MAX` bytes. + /// - Panics if the any of the reallocations causes an out-of-memory error. + /// /// # Safety /// - `current_capacity` must be the current capacity of this column (the capacity of `self.data`, `self.added_ticks`, `self.changed_tick`) /// - The caller should make sure their saved `capacity` value is updated to `new_capacity` after this operation. @@ -124,6 +161,10 @@ impl ThinColumn { /// Call [`alloc`](std::alloc::alloc) to allocate memory for this [`ThinColumn`] /// The caller should make sure their saved `capacity` value is updated to `new_capacity` after this operation. + /// + /// # Panics + /// - Panics if the any of the new capacity overflows `isize::MAX` bytes. + /// - Panics if the any of the allocations causes an out-of-memory error. pub(crate) fn alloc(&mut self, new_capacity: NonZeroUsize) { self.data.alloc(new_capacity); self.added_ticks.alloc(new_capacity); @@ -326,218 +367,8 @@ impl ThinColumn { .as_ref() .map(|changed_by| changed_by.as_slice(len)) } -} - -/// A type-erased contiguous container for data of a homogeneous type. -/// -/// Conceptually, a [`Column`] is very similar to a type-erased `Vec`. -/// It also stores the change detection ticks for its components, kept in two separate -/// contiguous buffers internally. An element shares its data across these buffers by using the -/// same index (i.e. the entity at row 3 has it's data at index 3 and its change detection ticks at index 3). -/// -/// Like many other low-level storage types, [`Column`] has a limited and highly unsafe -/// interface. It's highly advised to use higher level types and their safe abstractions -/// instead of working directly with [`Column`]. -#[derive(Debug)] -pub struct Column { - pub(super) data: BlobVec, - pub(super) added_ticks: Vec>, - pub(super) changed_ticks: Vec>, - changed_by: MaybeLocation>>>, -} - -impl Column { - /// Constructs a new [`Column`], configured with a component's layout and an initial `capacity`. - #[inline] - pub(crate) fn with_capacity(component_info: &ComponentInfo, capacity: usize) -> Self { - Column { - // SAFETY: component_info.drop() is valid for the types that will be inserted. - data: unsafe { BlobVec::new(component_info.layout(), component_info.drop(), capacity) }, - added_ticks: Vec::with_capacity(capacity), - changed_ticks: Vec::with_capacity(capacity), - changed_by: MaybeLocation::new_with(|| Vec::with_capacity(capacity)), - } - } - - /// Fetches the [`Layout`] for the underlying type. - #[inline] - pub fn item_layout(&self) -> Layout { - self.data.layout() - } - - /// Writes component data to the column at given row. - /// Assumes the slot is initialized, calls drop. - /// - /// # Safety - /// Assumes data has already been allocated for the given row. - #[inline] - pub(crate) unsafe fn replace( - &mut self, - row: TableRow, - data: OwningPtr<'_>, - change_tick: Tick, - caller: MaybeLocation, - ) { - debug_assert!(row.index() < self.len()); - self.data.replace_unchecked(row.index(), data); - *self.changed_ticks.get_unchecked_mut(row.index()).get_mut() = change_tick; - self.changed_by - .as_mut() - .map(|changed_by| changed_by.get_unchecked_mut(row.index()).get_mut()) - .assign(caller); - } - - /// Gets the current number of elements stored in the column. - #[inline] - pub fn len(&self) -> usize { - self.data.len() - } - - /// Checks if the column is empty. Returns `true` if there are no elements, `false` otherwise. - #[inline] - pub fn is_empty(&self) -> bool { - self.data.is_empty() - } - - /// Removes an element from the [`Column`]. - /// - /// - The value will be dropped if it implements [`Drop`]. - /// - This does not preserve ordering, but is O(1). - /// - This does not do any bounds checking. - /// - The element is replaced with the last element in the [`Column`]. - /// - /// # Safety - /// `row` must be within the range `[0, self.len())`. - #[inline] - pub(crate) unsafe fn swap_remove_unchecked(&mut self, row: TableRow) { - self.data.swap_remove_and_drop_unchecked(row.index()); - self.added_ticks.swap_remove(row.index()); - self.changed_ticks.swap_remove(row.index()); - self.changed_by - .as_mut() - .map(|changed_by| changed_by.swap_remove(row.index())); - } - - /// Removes an element from the [`Column`] and returns it and its change detection ticks. - /// 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`]. - /// - /// It's 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.) - /// - /// # Safety - /// `row` must be within the range `[0, self.len())`. - #[inline] - #[must_use = "The returned pointer should be used to dropped the removed component"] - pub(crate) unsafe fn swap_remove_and_forget_unchecked( - &mut self, - row: TableRow, - ) -> (OwningPtr<'_>, ComponentTicks, MaybeLocation) { - let data = self.data.swap_remove_and_forget_unchecked(row.index()); - let added = self.added_ticks.swap_remove(row.index()).into_inner(); - let changed = self.changed_ticks.swap_remove(row.index()).into_inner(); - let caller = self - .changed_by - .as_mut() - .map(|changed_by| changed_by.swap_remove(row.index()).into_inner()); - (data, ComponentTicks { added, changed }, caller) - } - - /// Pushes a new value onto the end of the [`Column`]. - /// - /// # Safety - /// `ptr` must point to valid data of this column's component type - pub(crate) unsafe fn push( - &mut self, - ptr: OwningPtr<'_>, - ticks: ComponentTicks, - caller: MaybeLocation, - ) { - self.data.push(ptr); - self.added_ticks.push(UnsafeCell::new(ticks.added)); - self.changed_ticks.push(UnsafeCell::new(ticks.changed)); - self.changed_by - .as_mut() - .zip(caller) - .map(|(changed_by, caller)| changed_by.push(UnsafeCell::new(caller))); - } - - /// Fetches the data pointer to the first element of the [`Column`]. - /// - /// The pointer is type erased, so using this function to fetch anything - /// other than the first element will require computing the offset using - /// [`Column::item_layout`]. - #[inline] - pub fn get_data_ptr(&self) -> Ptr<'_> { - self.data.get_ptr() - } - - /// Fetches the slice to the [`Column`]'s data cast to a given type. - /// - /// Note: The values stored within are [`UnsafeCell`]. - /// Users of this API must ensure that accesses to each individual element - /// adhere to the safety invariants of [`UnsafeCell`]. - /// - /// # Safety - /// The type `T` must be the type of the items in this column. - pub unsafe fn get_data_slice(&self) -> &[UnsafeCell] { - self.data.get_slice() - } - - /// Fetches the slice to the [`Column`]'s "added" change detection ticks. - /// - /// Note: The values stored within are [`UnsafeCell`]. - /// Users of this API must ensure that accesses to each individual element - /// adhere to the safety invariants of [`UnsafeCell`]. - #[inline] - pub fn get_added_ticks_slice(&self) -> &[UnsafeCell] { - &self.added_ticks - } - /// Fetches the slice to the [`Column`]'s "changed" change detection ticks. - /// - /// Note: The values stored within are [`UnsafeCell`]. - /// Users of this API must ensure that accesses to each individual element - /// adhere to the safety invariants of [`UnsafeCell`]. - #[inline] - pub fn get_changed_ticks_slice(&self) -> &[UnsafeCell] { - &self.changed_ticks - } - - /// Fetches a reference to the data and change detection ticks at `row`. - /// - /// Returns `None` if `row` is out of bounds. - #[inline] - pub fn get(&self, row: TableRow) -> Option<(Ptr<'_>, TickCells<'_>)> { - (row.index() < self.data.len()) - // SAFETY: The row is length checked before fetching the pointer. This is being - // accessed through a read-only reference to the column. - .then(|| unsafe { - ( - self.data.get_unchecked(row.index()), - TickCells { - added: self.added_ticks.get_unchecked(row.index()), - changed: self.changed_ticks.get_unchecked(row.index()), - }, - ) - }) - } - - /// Fetches a read-only reference to the data at `row`. - /// - /// Returns `None` if `row` is out of bounds. - #[inline] - pub fn get_data(&self, row: TableRow) -> Option> { - (row.index() < self.data.len()).then(|| { - // SAFETY: The row is length checked before fetching the pointer. This is being - // accessed through a read-only reference to the column. - unsafe { self.data.get_unchecked(row.index()) } - }) - } - - /// Fetches a read-only reference to the data at `row`. Unlike [`Column::get`] this does not + /// Fetches a read-only reference to the data at `row`. This does not /// do any bounds checking. /// /// # Safety @@ -545,150 +376,58 @@ impl Column { /// - no other mutable reference to the data of the same row can exist at the same time #[inline] pub unsafe fn get_data_unchecked(&self, row: TableRow) -> Ptr<'_> { - debug_assert!(row.index() < self.data.len()); self.data.get_unchecked(row.index()) } - /// Fetches a mutable reference to the data at `row`. - /// - /// Returns `None` if `row` is out of bounds. - #[inline] - pub fn get_data_mut(&mut self, row: TableRow) -> Option> { - (row.index() < self.data.len()).then(|| { - // SAFETY: The row is length checked before fetching the pointer. This is being - // accessed through an exclusive reference to the column. - unsafe { self.data.get_unchecked_mut(row.index()) } - }) - } - - /// Fetches the "added" change detection tick for the value at `row`. - /// - /// Returns `None` if `row` is out of bounds. - /// - /// Note: The values stored within are [`UnsafeCell`]. - /// Users of this API must ensure that accesses to each individual element - /// adhere to the safety invariants of [`UnsafeCell`]. - #[inline] - pub fn get_added_tick(&self, row: TableRow) -> Option<&UnsafeCell> { - self.added_ticks.get(row.index()) - } - - /// Fetches the "changed" change detection tick for the value at `row`. - /// - /// Returns `None` if `row` is out of bounds. + /// Fetches the calling location that last changed the value at `row`. /// - /// Note: The values stored within are [`UnsafeCell`]. - /// Users of this API must ensure that accesses to each individual element - /// adhere to the safety invariants of [`UnsafeCell`]. - #[inline] - pub fn get_changed_tick(&self, row: TableRow) -> Option<&UnsafeCell> { - self.changed_ticks.get(row.index()) - } - - /// Fetches the change detection ticks for the value at `row`. + /// This function does not do any bounds checking. /// - /// Returns `None` if `row` is out of bounds. + /// # Safety + /// `row` must be within the range `[0, self.len())`. #[inline] - pub fn get_ticks(&self, row: TableRow) -> Option { - if row.index() < self.data.len() { - // SAFETY: The size of the column has already been checked. - Some(unsafe { self.get_ticks_unchecked(row) }) - } else { - None - } + pub unsafe fn get_changed_by_unchecked( + &self, + row: TableRow, + ) -> MaybeLocation<&UnsafeCell<&'static Location<'static>>> { + self.changed_by + .as_ref() + .map(|changed_by| changed_by.get_unchecked(row.index())) } - /// Fetches the "added" change detection tick for the value at `row`. Unlike [`Column::get_added_tick`] - /// this function does not do any bounds checking. + /// Fetches the "added" change detection tick for the value at `row`. + /// This function does not do any bounds checking. /// /// # Safety /// `row` must be within the range `[0, self.len())`. #[inline] pub unsafe fn get_added_tick_unchecked(&self, row: TableRow) -> &UnsafeCell { - debug_assert!(row.index() < self.added_ticks.len()); self.added_ticks.get_unchecked(row.index()) } - /// Fetches the "changed" change detection tick for the value at `row`. Unlike [`Column::get_changed_tick`] - /// this function does not do any bounds checking. + /// Fetches the "changed" change detection tick for the value at `row` + /// This function does not do any bounds checking. /// /// # Safety /// `row` must be within the range `[0, self.len())`. #[inline] pub unsafe fn get_changed_tick_unchecked(&self, row: TableRow) -> &UnsafeCell { - debug_assert!(row.index() < self.changed_ticks.len()); self.changed_ticks.get_unchecked(row.index()) } - /// Fetches the change detection ticks for the value at `row`. Unlike [`Column::get_ticks`] - /// this function does not do any bounds checking. + /// Fetches the change detection ticks for the value at `row`. + /// This function does not do any bounds checking. /// /// # Safety /// `row` must be within the range `[0, self.len())`. #[inline] pub unsafe fn get_ticks_unchecked(&self, row: TableRow) -> ComponentTicks { - debug_assert!(row.index() < self.added_ticks.len()); - debug_assert!(row.index() < self.changed_ticks.len()); ComponentTicks { added: self.added_ticks.get_unchecked(row.index()).read(), changed: self.changed_ticks.get_unchecked(row.index()).read(), } } - /// Clears the column, removing all values. - /// - /// Note that this function has no effect on the allocated capacity of the [`Column`]> - pub fn clear(&mut self) { - self.data.clear(); - self.added_ticks.clear(); - self.changed_ticks.clear(); - self.changed_by.as_mut().map(Vec::clear); - } - - #[inline] - pub(crate) fn check_change_ticks(&mut self, check: CheckChangeTicks) { - for component_ticks in &mut self.added_ticks { - component_ticks.get_mut().check_tick(check); - } - for component_ticks in &mut self.changed_ticks { - component_ticks.get_mut().check_tick(check); - } - } - - /// Fetches the calling location that last changed the value at `row`. - /// - /// Returns `None` if `row` is out of bounds. - /// - /// Note: The values stored within are [`UnsafeCell`]. - /// Users of this API must ensure that accesses to each individual element - /// adhere to the safety invariants of [`UnsafeCell`]. - #[inline] - pub fn get_changed_by( - &self, - row: TableRow, - ) -> MaybeLocation>>> { - self.changed_by - .as_ref() - .map(|changed_by| changed_by.get(row.index())) - } - - /// Fetches the calling location that last changed the value at `row`. - /// - /// Unlike [`Column::get_changed_by`] this function does not do any bounds checking. - /// - /// # Safety - /// `row` must be within the range `[0, self.len())`. - #[inline] - pub unsafe fn get_changed_by_unchecked( - &self, - row: TableRow, - ) -> MaybeLocation<&UnsafeCell<&'static Location<'static>>> { - self.changed_by.as_ref().map(|changed_by| { - debug_assert!(row.index() < changed_by.len()); - changed_by.get_unchecked(row.index()) - }) - } - /// Returns the drop function for elements of the column, /// or `None` if they don't need to be dropped. #[inline] diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index 548ba82103f8a..da8d0da394a81 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -3,14 +3,13 @@ use crate::{ component::{CheckChangeTicks, ComponentId, ComponentInfo, ComponentTicks, Components, Tick}, entity::Entity, query::DebugCheckedUnwrap, - storage::{blob_vec::BlobVec, ImmutableSparseSet, SparseSet}, + storage::{AbortOnPanic, ImmutableSparseSet, SparseSet}, }; use alloc::{boxed::Box, vec, vec::Vec}; use bevy_platform::collections::HashMap; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; pub use column::*; use core::{ - alloc::Layout, cell::UnsafeCell, num::NonZeroUsize, ops::{Index, IndexMut}, @@ -183,15 +182,6 @@ pub struct Table { entities: Vec, } -struct AbortOnPanic; - -impl Drop for AbortOnPanic { - fn drop(&mut self) { - // Panicking while unwinding will force an abort. - panic!("Aborting due to allocator error"); - } -} - impl Table { /// Fetches a read-only slice of the entities stored within the [`Table`]. #[inline] @@ -253,6 +243,10 @@ impl Table { /// the caller's responsibility to drop them. Failure to do so may result in resources not /// being released (i.e. files handles not being released, memory leaks, etc.) /// + /// # Panics + /// - Panics if the move forces a reallocation, and any of the new capacity overflows `isize::MAX` bytes. + /// - Panics if the move forces a reallocation, and any of the new the reallocations causes an out-of-memory error. + /// /// # Safety /// - `row` must be in-bounds pub(crate) unsafe fn move_to_and_forget_missing_unchecked( @@ -292,6 +286,10 @@ impl Table { /// Returns the index of the new row in `new_table` and the entity in this table swapped in /// to replace it (if an entity was swapped in). /// + /// # Panics + /// - Panics if the move forces a reallocation, and any of the new capacity overflows `isize::MAX` bytes. + /// - Panics if the move forces a reallocation, and any of the new the reallocations causes an out-of-memory error. + /// /// # Safety /// row must be in-bounds pub(crate) unsafe fn move_to_and_drop_missing_unchecked( @@ -330,6 +328,10 @@ impl Table { /// Returns the index of the new row in `new_table` and the entity in this table swapped in /// to replace it (if an entity was swapped in). /// + /// # Panics + /// - Panics if the move forces a reallocation, and any of the new capacity overflows `isize::MAX` bytes. + /// - Panics if the move forces a reallocation, and any of the new the reallocations causes an out-of-memory error. + /// /// # Safety /// - `row` must be in-bounds /// - `new_table` must contain every component this table has @@ -528,6 +530,10 @@ impl Table { /// Allocate memory for the columns in the [`Table`] /// + /// # Panics + /// - Panics if any of the new capacity overflows `isize::MAX` bytes. + /// - Panics if any of the new allocations causes an out-of-memory error. + /// /// The current capacity of the columns should be 0, if it's not 0, then the previous data will be overwritten and leaked. fn alloc_columns(&mut self, new_capacity: NonZeroUsize) { // If any of these allocations trigger an unwind, the wrong capacity will be used while dropping this table - UB. @@ -542,6 +548,10 @@ impl Table { /// Reallocate memory for the columns in the [`Table`] /// + /// # Panics + /// - Panics if any of the new capacities overflows `isize::MAX` bytes. + /// - Panics if any of the new reallocations causes an out-of-memory error. + /// /// # Safety /// - `current_column_capacity` is indeed the capacity of the columns unsafe fn realloc_columns( @@ -566,6 +576,10 @@ impl Table { /// Allocates space for a new entity /// + /// # Panics + /// - Panics if the allocation forces a reallocation and the new capacities overflows `isize::MAX` bytes. + /// - Panics if the allocation forces a reallocation and causes an out-of-memory error. + /// /// # Safety /// /// The allocated row must be written to immediately with valid values in each column @@ -643,6 +657,9 @@ impl Table { } /// Clears all of the stored components in the [`Table`]. + /// + /// # Panics + /// - Panics if any of the components in any of the columns panics while being dropped. pub(crate) fn clear(&mut self) { let len = self.entity_count() as usize; // We must clear the entities first, because in the drop function causes a panic, it will result in a double free of the columns. @@ -822,7 +839,7 @@ impl Drop for Table { let cap = self.capacity(); self.entities.clear(); for col in self.columns.values_mut() { - // SAFETY: `cap` and `len` are correct + // SAFETY: `cap` and `len` are correct. `col` is never accessed again after this call. unsafe { col.drop(cap, len); } diff --git a/crates/bevy_ecs/src/storage/thin_array_ptr.rs b/crates/bevy_ecs/src/storage/thin_array_ptr.rs index 90163440297c8..6b9f98ca603bd 100644 --- a/crates/bevy_ecs/src/storage/thin_array_ptr.rs +++ b/crates/bevy_ecs/src/storage/thin_array_ptr.rs @@ -16,6 +16,7 @@ use core::{ /// memory leaks, [`drop`](Self::drop) must be called when no longer in use. /// /// [`Vec`]: alloc::vec::Vec +#[derive(Debug)] pub struct ThinArrayPtr { data: NonNull, #[cfg(debug_assertions)] @@ -48,6 +49,10 @@ impl ThinArrayPtr { } /// Create a new [`ThinArrayPtr`] with a given capacity. If the `capacity` is 0, this will no allocate any memory. + /// + /// # Panics + /// - Panics if the new capacity overflows `isize::MAX` bytes. + /// - Panics if the allocation causes an out-of-memory error. #[inline] pub fn with_capacity(capacity: usize) -> Self { let mut arr = Self::empty(); @@ -63,6 +68,10 @@ impl ThinArrayPtr { /// The caller should update their saved `capacity` value to reflect the fact that it was changed /// /// # Panics + /// - Panics if the new capacity overflows `isize::MAX` bytes. + /// - Panics if the allocation causes an out-of-memory error. + /// + /// # Panics /// - Panics if the new capacity overflows `usize` pub fn alloc(&mut self, capacity: NonZeroUsize) { self.set_capacity(capacity.get()); @@ -80,7 +89,8 @@ impl ThinArrayPtr { /// Reallocate memory for the array, this should only be used if a previous allocation for this array has been made (capacity > 0). /// /// # Panics - /// - Panics if the new capacity overflows `usize` + /// - Panics if the new capacity overflows `isize::MAX` bytes + /// - Panics if the allocation causes an out-of-memory error. /// /// # Safety /// - The current capacity is indeed greater than 0