Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

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

Already on GitHub? Sign in to your account

[Merged by Bors] - Directly copy moved Table components to the target location #5056

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub use iter::*;
pub use state::*;

#[allow(unreachable_code)]
unsafe fn debug_checked_unreachable() -> ! {
pub(crate) unsafe fn debug_checked_unreachable() -> ! {
#[cfg(debug_assertions)]
unreachable!();
std::hint::unreachable_unchecked();
Expand Down
23 changes: 23 additions & 0 deletions crates/bevy_ecs/src/storage/blob_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,29 @@ impl BlobVec {
OwningPtr::new(self.swap_scratch)
}

/// Removes the value at `index` and copies the value stored into `ptr`.
/// Does not do any bounds checking on `index`.
///
/// # Safety
/// It is the caller's responsibility to ensure that `index` is < `self.len()`
/// and that `self[index]` has been properly initialized.
#[inline]
pub unsafe fn remove_unchecked(&mut self, index: usize, ptr: PtrMut<'_>) {
debug_assert!(index < self.len());
let last = self.len - 1;
james7132 marked this conversation as resolved.
Show resolved Hide resolved
std::ptr::copy_nonoverlapping::<u8>(
james7132 marked this conversation as resolved.
Show resolved Hide resolved
self.get_unchecked_mut(index).as_ptr(),
ptr.as_ptr(),
self.item_layout.size(),
);
std::ptr::copy::<u8>(
james7132 marked this conversation as resolved.
Show resolved Hide resolved
self.get_unchecked_mut(last).as_ptr(),
self.get_unchecked_mut(index).as_ptr(),
self.item_layout.size(),
);
self.len -= 1;
james7132 marked this conversation as resolved.
Show resolved Hide resolved
}

/// # Safety
/// It is the caller's responsibility to ensure that `index` is < self.len()
#[inline]
Expand Down
41 changes: 34 additions & 7 deletions crates/bevy_ecs/src/storage/table.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::{
component::{ComponentId, ComponentInfo, ComponentTicks, Components},
entity::Entity,
query::debug_checked_unreachable,
storage::{BlobVec, SparseSet},
};
use bevy_ptr::{OwningPtr, Ptr, PtrMut};
Expand Down Expand Up @@ -119,6 +120,30 @@ impl Column {
(data, ticks)
}

/// Removes the element from `other` at `src_row` and inserts it
/// into the current column to initialize the values at `dst_row`.
/// Does not do any bounds checking.
///
/// # Safety
///
/// - `other` must have the same data layout as `self`
/// - `src_row` must be in bounds for `other`
/// - `dst_row` must be in bounds for `self`
/// - `other[src_row]` must be initialized to a valid value.
/// - `self[dst_row]` must not be initalized yet.
#[inline]
pub(crate) unsafe fn initialize_from_unchecked(
&mut self,
other: &mut Column,
src_row: usize,
dst_row: usize,
) {
debug_assert!(self.data.layout() == other.data.layout());
let ptr = self.data.get_unchecked_mut(dst_row);
other.data.remove_unchecked(src_row, ptr);
*self.ticks.get_unchecked_mut(dst_row) = other.ticks.swap_remove(src_row);
}

// # Safety
// - ptr must point to valid data of this column's component type
pub(crate) unsafe fn push(&mut self, ptr: OwningPtr<'_>, ticks: ComponentTicks) {
Expand Down Expand Up @@ -253,9 +278,11 @@ impl Table {
let is_last = row == self.entities.len() - 1;
let new_row = new_table.allocate(self.entities.swap_remove(row));
for (component_id, column) in self.columns.iter_mut() {
let (data, ticks) = column.swap_remove_and_forget_unchecked(row);
if let Some(new_column) = new_table.get_column_mut(*component_id) {
new_column.initialize(new_row, data, ticks);
new_column.initialize_from_unchecked(column, row, new_row);
} else {
// It's the caller's responsibility to drop these cases.
let (_, _) = column.swap_remove_and_forget_unchecked(row);
}
}
TableMoveResult {
Expand Down Expand Up @@ -284,8 +311,7 @@ impl Table {
let new_row = new_table.allocate(self.entities.swap_remove(row));
for (component_id, column) in self.columns.iter_mut() {
if let Some(new_column) = new_table.get_column_mut(*component_id) {
let (data, ticks) = column.swap_remove_and_forget_unchecked(row);
new_column.initialize(new_row, data, ticks);
new_column.initialize_from_unchecked(column, row, new_row);
} else {
column.swap_remove_unchecked(row);
}
Expand Down Expand Up @@ -315,9 +341,10 @@ impl Table {
let is_last = row == self.entities.len() - 1;
let new_row = new_table.allocate(self.entities.swap_remove(row));
for (component_id, column) in self.columns.iter_mut() {
let new_column = new_table.get_column_mut(*component_id).unwrap();
let (data, ticks) = column.swap_remove_and_forget_unchecked(row);
new_column.initialize(new_row, data, ticks);
new_table
.get_column_mut(*component_id)
.unwrap_or_else(|| debug_checked_unreachable())
.initialize_from_unchecked(column, row, new_row);
}
TableMoveResult {
new_row,
Expand Down