Skip to content

Use stabilized Layout::repeat #24006

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alice-i-cecile merged 1 commit into from
Apr 28, 2026
Merged

Use stabilized Layout::repeat #24006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion crates/bevy_ecs/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ repository = "https://github.com/bevyengine/bevy"
license = "MIT OR Apache-2.0"
keywords = ["ecs", "game", "bevy"]
categories = ["game-engines", "data-structures"]
rust-version = "1.89.0"
rust-version = "1.95.0"

[features]
default = ["std", "bevy_reflect", "async_executor", "backtrace"]
Expand Down
13 changes: 13 additions & 0 deletions crates/bevy_ecs/src/component/info.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ pub struct ComponentDescriptor {
// actually Send + Sync
is_send_and_sync: bool,
type_id: Option<TypeId>,
// SAFETY: This must always have `size()` that is a multiple of `align()`.
// `BlobArray` relies on that to calculate byte offsets as a multiple of `size()`.
layout: Layout,
// SAFETY: this function must be safe to call with pointers pointing to items of the type
// this descriptor describes.
Expand Down Expand Up @@ -264,6 +266,7 @@ impl ComponentDescriptor {
storage_type: T::STORAGE_TYPE,
is_send_and_sync: true,
type_id: Some(TypeId::of::<T>()),
// `T` is a rust type, so the layout will have `size()` as a multiple of `align()`
layout: Layout::new::<T>(),
drop: needs_drop::<T>().then_some(Self::drop_ptr::<T> as _),
mutable: T::Mutability::MUTABLE,
Expand All @@ -274,6 +277,10 @@ impl ComponentDescriptor {

/// Create a new `ComponentDescriptor`.
///
/// # Panics
///
/// Panics if `layout` does not have a `size()` that is a multiple of its `alignment()`.
///
/// # Safety
/// - the `drop` fn must be usable on a pointer with a value of the layout `layout`
/// - the component type must be safe to access from any thread (Send + Sync in rust terms)
Expand All @@ -287,6 +294,11 @@ impl ComponentDescriptor {
clone_behavior: ComponentCloneBehavior,
relationship_accessor: Option<RelationshipAccessorInitializer>,
) -> Self {
assert_eq!(
layout.pad_to_align(),
layout,
"Layout size must be a multiple of its alignment. Consider calling `pad_to_align()`."
);
Self {
name: name.into().into(),
storage_type,
Expand Down Expand Up @@ -314,6 +326,7 @@ impl ComponentDescriptor {
storage_type,
is_send_and_sync: false,
type_id: Some(TypeId::of::<T>()),
// `T` is a rust type, so the layout will have `size()` as a multiple of `align()`
layout: Layout::new::<T>(),
drop: needs_drop::<T>().then_some(Self::drop_ptr::<T> as _),
mutable: true,
Expand Down
125 changes: 26 additions & 99 deletions crates/bevy_ecs/src/storage/blob_array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use bevy_ptr::{OwningPtr, Ptr, PtrMut};
use bevy_utils::OnDrop;
use core::{alloc::Layout, cell::UnsafeCell, num::NonZeroUsize, ptr::NonNull};

use crate::query::DebugCheckedUnwrap;

/// 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
Expand All @@ -12,6 +14,10 @@ use core::{alloc::Layout, cell::UnsafeCell, num::NonZeroUsize, ptr::NonNull};
/// This type is reliant on its owning type to store the capacity and length information.
#[derive(Debug)]
pub(super) struct BlobArray {
/// The layout of the data.
/// This always has `size()` as a multiple of `align()`,
/// meaning we can use `repeat_packed` for layout and can
/// index the array by multiplying `size()` by the index.
item_layout: Layout,
// the `data` ptr's layout is always `array_layout(item_layout, capacity)`
data: NonNull<u8>,
Expand All @@ -31,8 +37,10 @@ impl BlobArray {
/// processes typically associated with the stored element.
///
/// # Safety
/// `drop` should be safe to call with an [`OwningPtr`] pointing to any item that's been placed into this [`BlobArray`].
/// If `drop` is `None`, the items will be leaked. This should generally be set as None based on [`needs_drop`].
/// - `drop` should be safe to call with an [`OwningPtr`] pointing to any item that's been placed into this [`BlobArray`].
/// If `drop` is `None`, the items will be leaked. This should generally be set as None based on [`needs_drop`].
/// - `item_layout.size()` must be a multiple of `item_layout.align()`.
/// Note that this is true for all rust types, but not all `Layout` values.
///
/// [`needs_drop`]: std::mem::needs_drop
pub unsafe fn with_capacity(
Expand All @@ -42,6 +50,12 @@ impl BlobArray {
) -> Self {
if capacity == 0 {
let align = NonZeroUsize::new(item_layout.align()).expect("alignment must be > 0");
// Indexing operations require that the size be a multiple of the alignment
debug_assert_eq!(
item_layout.pad_to_align(),
item_layout,
"Layout size must be a multiple of its alignment"
);

// Create a dangling pointer with the given alignment.
let data = NonNull::without_provenance(align);
Expand Down Expand Up @@ -204,8 +218,8 @@ impl BlobArray {
if cap != 0 {
self.clear(len);
if !self.is_zst() {
let layout =
array_layout(&self.item_layout, cap).expect("array layout should be valid");
let layout = self.item_layout.repeat_packed(cap);
let layout = layout.expect("array layout should be valid");
alloc::alloc::dealloc(self.data.as_ptr().cast(), layout);
}
#[cfg(debug_assertions)]
Expand Down Expand Up @@ -246,8 +260,8 @@ impl BlobArray {
#[cfg(debug_assertions)]
debug_assert_eq!(self.capacity, 0);
if !self.is_zst() {
let new_layout = array_layout(&self.item_layout, capacity.get())
.expect("array layout should be valid");
let new_layout = self.item_layout.repeat_packed(capacity.get());
let new_layout = new_layout.expect("array layout should be valid");
// SAFETY: layout has non-zero size because capacity > 0, and the blob isn't ZST (`self.is_zst` == false)
let new_data = unsafe { alloc::alloc::alloc(new_layout) };
self.data = NonNull::new(new_data).unwrap_or_else(|| handle_alloc_error(new_layout));
Expand Down Expand Up @@ -277,20 +291,21 @@ impl BlobArray {
#[cfg(debug_assertions)]
debug_assert_eq!(self.capacity, current_capacity.get());
if !self.is_zst() {
let new_layout = array_layout(&self.item_layout, new_capacity.get())
.expect("array layout should be valid");
let new_layout = self.item_layout.repeat_packed(new_capacity.get());
let new_layout = new_layout.expect("array layout should be valid");
let layout = self.item_layout.repeat_packed(current_capacity.get());
// SAFETY:
// - ptr was be allocated via this allocator
// - the layout used to previously allocate this array is equivalent to `array_layout(&self.item_layout, current_capacity.get())`
// - the layout used to previously allocate this array is equivalent to `self.item_layout.repeat_packed(current_capacity.get())`
// - `item_layout.size() > 0` (`self.is_zst`==false) 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 align, the rounding cannot happen
// here and the overflow is handled in `array_layout`
// here and the overflow is handled in `Layout::repeat_packed`
let new_data = unsafe {
alloc::alloc::realloc(
self.get_ptr_mut().as_ptr(),
// SAFETY: This is the Layout of the current array, it must be valid, if it hadn't have been, there would have been a panic on a previous allocation
array_layout_unchecked(&self.item_layout, current_capacity.get()),
layout.debug_checked_unwrap(),
new_layout.size(),
)
};
Expand Down Expand Up @@ -493,94 +508,6 @@ impl BlobArray {
}
}

/// From <https://doc.rust-lang.org/beta/src/core/alloc/layout.rs.html>
pub(super) fn array_layout(layout: &Layout, n: usize) -> Option<Layout> {
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 <https://doc.rust-lang.org/beta/src/core/alloc/layout.rs.html>
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 <https://doc.rust-lang.org/beta/src/core/alloc/layout.rs.html>
/// # 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 <https://doc.rust-lang.org/beta/src/core/alloc/layout.rs.html>
/// # 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 <https://doc.rust-lang.org/beta/src/core/alloc/layout.rs.html>
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::*;
Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_ecs/src/storage/non_send.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,9 @@ impl NonSends {
) -> &mut NonSendData {
self.non_sends.get_or_insert_with(component_id, || {
let component_info = components.get_info(component_id).unwrap();
// SAFETY: component_info.drop() is valid for the types that will be inserted.
// SAFETY:
// * component_info.drop() is valid for the types that will be inserted.
// * `ComponentInfo` ensures that `layout().size()` is a multiple of `layout().align()`
let data = unsafe {
BlobArray::with_capacity(component_info.layout(), component_info.drop(), 1)
};
Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_ecs/src/storage/table/column.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ impl Column {
/// Create a new [`Column`] with the given `capacity`.
pub fn with_capacity(component_info: &ComponentInfo, capacity: usize) -> Self {
Self {
// SAFETY: The components stored in this columns will match the information in `component_info`
// SAFETY:
// * The components stored in this columns will match the information in `component_info`
// * `ComponentInfo` ensures that `layout().size()` is a multiple of `layout().align()`
data: unsafe {
BlobArray::with_capacity(component_info.layout(), component_info.drop(), capacity)
},
Expand Down