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] - Extend EntityLocation with TableId and TableRow #6681

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 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
26 changes: 14 additions & 12 deletions crates/bevy_ecs/src/archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,23 @@ use std::{

#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
#[repr(transparent)]
pub struct ArchetypeId(usize);
pub struct ArchetypeId(u32);

impl ArchetypeId {
pub const EMPTY: ArchetypeId = ArchetypeId(0);
/// # Safety:
///
/// This must always have an all-1s bit pattern to ensure soundness in fast entity id space allocation.
pub const INVALID: ArchetypeId = ArchetypeId(usize::MAX);
pub const INVALID: ArchetypeId = ArchetypeId(u32::MAX);

#[inline]
pub const fn new(index: usize) -> Self {
ArchetypeId(index)
ArchetypeId(index as u32)
}

#[inline]
pub fn index(self) -> usize {
self.0
self.0 as usize
}
}

Expand Down Expand Up @@ -275,8 +275,8 @@ impl Archetype {
}

#[inline]
pub(crate) fn set_entity_table_row(&mut self, index: usize, table_row: usize) {
self.entities[index].table_row = table_row;
pub(crate) fn set_entity_table_row(&mut self, index: u32, table_row: usize) {
self.entities[index as usize].table_row = table_row;
}

/// # Safety
Expand All @@ -287,7 +287,9 @@ impl Archetype {

EntityLocation {
archetype_id: self.id,
index: self.entities.len() - 1,
archetype_index: (self.entities.len() - 1) as u32,
table_id: self.table_id,
table_row: table_row as u32,
}
}

Expand All @@ -297,14 +299,14 @@ impl Archetype {

/// Removes the entity at `index` by swapping it out. Returns the table row the entity is stored
/// in.
pub(crate) fn swap_remove(&mut self, index: usize) -> ArchetypeSwapRemoveResult {
let is_last = index == self.entities.len() - 1;
let entity = self.entities.swap_remove(index);
pub(crate) fn swap_remove(&mut self, index: u32) -> ArchetypeSwapRemoveResult {
let is_last = index as usize == self.entities.len() - 1;
let entity = self.entities.swap_remove(index as usize);
ArchetypeSwapRemoveResult {
swapped_entity: if is_last {
None
} else {
Some(self.entities[index].entity)
Some(self.entities[index as usize].entity)
},
table_row: entity.table_row,
}
Expand Down Expand Up @@ -491,7 +493,7 @@ impl Archetypes {
.archetype_ids
.entry(archetype_identity)
.or_insert_with(move || {
let id = ArchetypeId(archetypes.len());
let id = ArchetypeId::new(archetypes.len());
let table_start = *archetype_component_count;
*archetype_component_count += table_components.len();
let table_archetype_components =
Expand Down
14 changes: 5 additions & 9 deletions crates/bevy_ecs/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,13 +528,9 @@ impl<'a, 'b> BundleInserter<'a, 'b> {
pub unsafe fn insert<T: Bundle>(
&mut self,
entity: Entity,
archetype_index: usize,
location: EntityLocation,
bundle: T,
) -> EntityLocation {
let location = EntityLocation {
index: archetype_index,
archetype_id: self.archetype.id(),
};
match &mut self.result {
InsertBundleResult::SameArchetype => {
// PERF: this could be looked up during Inserter construction and stored (but borrowing makes this nasty)
Expand All @@ -548,14 +544,14 @@ impl<'a, 'b> BundleInserter<'a, 'b> {
self.sparse_sets,
add_bundle,
entity,
self.archetype.entity_table_row(archetype_index),
location.table_row as usize,
self.change_tick,
bundle,
);
location
}
InsertBundleResult::NewArchetypeSameTable { new_archetype } => {
let result = self.archetype.swap_remove(location.index);
let result = self.archetype.swap_remove(location.archetype_index);
if let Some(swapped_entity) = result.swapped_entity {
self.entities.meta[swapped_entity.index as usize].location = location;
}
Expand Down Expand Up @@ -583,7 +579,7 @@ impl<'a, 'b> BundleInserter<'a, 'b> {
new_archetype,
new_table,
} => {
let result = self.archetype.swap_remove(location.index);
let result = self.archetype.swap_remove(location.archetype_index);
if let Some(swapped_entity) = result.swapped_entity {
self.entities.meta[swapped_entity.index as usize].location = location;
}
Expand Down Expand Up @@ -611,7 +607,7 @@ impl<'a, 'b> BundleInserter<'a, 'b> {
};

swapped_archetype
.set_entity_table_row(swapped_location.index, result.table_row);
.set_entity_table_row(swapped_location.archetype_index, result.table_row);
}

// PERF: this could be looked up during Inserter construction and stored (but borrowing makes this nasty)
Expand Down
13 changes: 10 additions & 3 deletions crates/bevy_ecs/src/entity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ mod map_entities;

pub use map_entities::*;

use crate::{archetype::ArchetypeId, storage::SparseSetIndex};
use crate::{
archetype::ArchetypeId,
storage::{SparseSetIndex, TableId},
};
use serde::{Deserialize, Serialize};
use std::{convert::TryFrom, fmt, mem, sync::atomic::Ordering};

Expand Down Expand Up @@ -676,7 +679,9 @@ impl EntityMeta {
generation: 0,
location: EntityLocation {
archetype_id: ArchetypeId::INVALID,
index: usize::MAX, // dummy value, to be filled in
archetype_index: u32::MAX, // dummy value, to be filled in
james7132 marked this conversation as resolved.
Show resolved Hide resolved
table_id: TableId::INVALID,
table_row: u32::MAX, // dummy value, to be filled in
},
};
}
Expand All @@ -689,7 +694,9 @@ pub struct EntityLocation {
pub archetype_id: ArchetypeId,

/// The index of the entity in the archetype
pub index: usize,
pub archetype_index: u32,
james7132 marked this conversation as resolved.
Show resolved Hide resolved
pub table_id: TableId,
pub table_row: u32,
}

#[cfg(test)]
Expand Down
11 changes: 7 additions & 4 deletions crates/bevy_ecs/src/query/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ where
.archetypes
.get(location.archetype_id)
.debug_checked_unwrap();
let table = self.tables.get(archetype.table_id()).debug_checked_unwrap();
let table = self.tables.get(location.table_id).debug_checked_unwrap();

// SAFETY: `archetype` is from the world that `fetch/filter` were created for,
// `fetch_state`/`filter_state` are the states that `fetch/filter` were initialized with
Expand All @@ -176,12 +176,15 @@ where
table,
);

let table_row = archetype.entity_table_row(location.index);
// SAFETY: set_archetype was called prior.
// `location.index` is an archetype index row in range of the current archetype, because if it was not, the match above would have `continue`d
if F::filter_fetch(&mut self.filter, entity, table_row) {
if F::filter_fetch(&mut self.filter, entity, location.table_row as usize) {
james7132 marked this conversation as resolved.
Show resolved Hide resolved
// SAFETY: set_archetype was called prior, `location.index` is an archetype index in range of the current archetype
return Some(Q::fetch(&mut self.fetch, entity, table_row));
return Some(Q::fetch(
&mut self.fetch,
entity,
location.table_row as usize,
james7132 marked this conversation as resolved.
Show resolved Hide resolved
));
}
}
None
Expand Down
7 changes: 3 additions & 4 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,17 +418,16 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
let mut fetch = Q::init_fetch(world, &self.fetch_state, last_change_tick, change_tick);
let mut filter = F::init_fetch(world, &self.filter_state, last_change_tick, change_tick);

let table_row = archetype.entity_table_row(location.index);
let table = world
.storages()
.tables
.get(archetype.table_id())
.get(location.table_id)
.debug_checked_unwrap();
Q::set_archetype(&mut fetch, &self.fetch_state, archetype, table);
F::set_archetype(&mut filter, &self.filter_state, archetype, table);

if F::filter_fetch(&mut filter, entity, table_row) {
Ok(Q::fetch(&mut fetch, entity, table_row))
if F::filter_fetch(&mut filter, entity, location.table_row as usize) {
Ok(Q::fetch(&mut fetch, entity, location.table_row as usize))
} else {
Err(QueryEntityError::QueryDoesNotMatch(entity))
}
Expand Down
10 changes: 6 additions & 4 deletions crates/bevy_ecs/src/storage/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,19 @@ use std::{
};

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct TableId(usize);
pub struct TableId(u32);
james7132 marked this conversation as resolved.
Show resolved Hide resolved

impl TableId {
pub(crate) const INVALID: TableId = TableId(u32::MAX);

#[inline]
pub fn new(index: usize) -> Self {
james7132 marked this conversation as resolved.
Show resolved Hide resolved
TableId(index)
TableId(index as u32)
}

#[inline]
pub fn index(self) -> usize {
self.0
self.0 as usize
}

#[inline]
Expand Down Expand Up @@ -585,7 +587,7 @@ impl Tables {
table.add_column(components.get_info_unchecked(*component_id));
}
tables.push(table.build());
(component_ids.to_vec(), TableId(tables.len() - 1))
(component_ids.to_vec(), TableId::new(tables.len() - 1))
});

*value
Expand Down
40 changes: 16 additions & 24 deletions crates/bevy_ecs/src/world/entity_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ impl<'w> EntityMut<'w> {
);
// SAFETY: location matches current entity. `T` matches `bundle_info`
unsafe {
self.location = bundle_inserter.insert(self.entity, self.location.index, bundle);
self.location = bundle_inserter.insert(self.entity, self.location, bundle);
}

self
Expand Down Expand Up @@ -310,7 +310,6 @@ impl<'w> EntityMut<'w> {
return None;
}

let old_archetype = &mut archetypes[old_location.archetype_id];
let mut bundle_components = bundle_info.component_ids.iter().cloned();
let entity = self.entity;
// SAFETY: bundle components are iterated in order, which guarantees that the component type
Expand All @@ -322,7 +321,6 @@ impl<'w> EntityMut<'w> {
take_component(
components,
storages,
old_archetype,
removed_components,
component_id,
entity,
Expand Down Expand Up @@ -368,7 +366,7 @@ impl<'w> EntityMut<'w> {
new_archetype_id: ArchetypeId,
) {
let old_archetype = &mut archetypes[old_archetype_id];
let remove_result = old_archetype.swap_remove(old_location.index);
let remove_result = old_archetype.swap_remove(old_location.archetype_index);
if let Some(swapped_entity) = remove_result.swapped_entity {
entities.meta[swapped_entity.index as usize].location = old_location;
}
Expand Down Expand Up @@ -397,7 +395,7 @@ impl<'w> EntityMut<'w> {
if let Some(swapped_entity) = move_result.swapped_entity {
let swapped_location = entities.get(swapped_entity).unwrap();
archetypes[swapped_location.archetype_id]
.set_entity_table_row(swapped_location.index, old_table_row);
.set_entity_table_row(swapped_location.archetype_index, old_table_row);
}

new_location
Expand Down Expand Up @@ -498,7 +496,7 @@ impl<'w> EntityMut<'w> {
.get_or_insert_with(component_id, Vec::new);
removed_components.push(self.entity);
}
let remove_result = archetype.swap_remove(location.index);
let remove_result = archetype.swap_remove(location.archetype_index);
if let Some(swapped_entity) = remove_result.swapped_entity {
world.entities.meta[swapped_entity.index as usize].location = location;
}
Expand All @@ -517,7 +515,7 @@ impl<'w> EntityMut<'w> {
if let Some(moved_entity) = moved_entity {
let moved_location = world.entities.get(moved_entity).unwrap();
world.archetypes[moved_location.archetype_id]
.set_entity_table_row(moved_location.index, table_row);
.set_entity_table_row(moved_location.archetype_index, table_row);
}
}

Expand Down Expand Up @@ -605,16 +603,14 @@ pub(crate) unsafe fn get_component(
entity: Entity,
location: EntityLocation,
) -> Option<Ptr<'_>> {
let archetype = &world.archetypes[location.archetype_id];
// SAFETY: component_id exists and is therefore valid
let component_info = world.components.get_info_unchecked(component_id);
match component_info.storage_type() {
StorageType::Table => {
let table = &world.storages.tables[archetype.table_id()];
let table = &world.storages.tables[location.table_id];
let components = table.get_column(component_id)?;
let table_row = archetype.entity_table_row(location.index);
// SAFETY: archetypes only store valid table_rows and the stored component type is T
Some(components.get_data_unchecked(table_row))
Some(components.get_data_unchecked(location.table_row as usize))
}
StorageType::SparseSet => world
.storages
Expand All @@ -636,17 +632,15 @@ unsafe fn get_component_and_ticks(
entity: Entity,
location: EntityLocation,
) -> Option<(Ptr<'_>, &UnsafeCell<ComponentTicks>)> {
let archetype = &world.archetypes[location.archetype_id];
let component_info = world.components.get_info_unchecked(component_id);
match component_info.storage_type() {
StorageType::Table => {
let table = &world.storages.tables[archetype.table_id()];
let table = &world.storages.tables[location.table_id];
let components = table.get_column(component_id)?;
let table_row = archetype.entity_table_row(location.index);
// SAFETY: archetypes only store valid table_rows and the stored component type is T
Some((
components.get_data_unchecked(table_row),
components.get_ticks_unchecked(table_row),
components.get_data_unchecked(location.table_row as usize),
james7132 marked this conversation as resolved.
Show resolved Hide resolved
components.get_ticks_unchecked(location.table_row as usize),
))
}
StorageType::SparseSet => world
Expand All @@ -664,15 +658,13 @@ unsafe fn get_ticks(
entity: Entity,
location: EntityLocation,
) -> Option<&UnsafeCell<ComponentTicks>> {
let archetype = &world.archetypes[location.archetype_id];
let component_info = world.components.get_info_unchecked(component_id);
match component_info.storage_type() {
StorageType::Table => {
let table = &world.storages.tables[archetype.table_id()];
let table = &world.storages.tables[location.table_id];
let components = table.get_column(component_id)?;
let table_row = archetype.entity_table_row(location.index);
// SAFETY: archetypes only store valid table_rows and the stored component type is T
Some(components.get_ticks_unchecked(table_row))
Some(components.get_ticks_unchecked(location.table_row as usize))
}
StorageType::SparseSet => world
.storages
Expand All @@ -697,7 +689,6 @@ unsafe fn get_ticks(
unsafe fn take_component<'a>(
components: &Components,
storages: &'a mut Storages,
archetype: &Archetype,
removed_components: &mut SparseSet<ComponentId, Vec<Entity>>,
component_id: ComponentId,
entity: Entity,
Expand All @@ -708,12 +699,13 @@ unsafe fn take_component<'a>(
removed_components.push(entity);
match component_info.storage_type() {
StorageType::Table => {
let table = &mut storages.tables[archetype.table_id()];
let table = &mut storages.tables[location.table_id];
// SAFETY: archetypes will always point to valid columns
let components = table.get_column_mut(component_id).unwrap();
let table_row = archetype.entity_table_row(location.index);
// SAFETY: archetypes only store valid table_rows and the stored component type is T
components.get_data_unchecked_mut(table_row).promote()
components
.get_data_unchecked_mut(location.table_row as usize)
james7132 marked this conversation as resolved.
Show resolved Hide resolved
.promote()
}
StorageType::SparseSet => storages
.sparse_sets
Expand Down
Loading