Skip to content

Commit

Permalink
Remove useless access to archetype in UnsafeWorldCell::fetch_table (b…
Browse files Browse the repository at this point in the history
…evyengine#7665)

# Objective

- bevyengine#6402 changed `World::fetch_table` (now `UnsafeWorldCell::fetch_table`) to access the archetype in order to get the `table_id` and `table_row` of the entity involved. However this is useless since those were already present in the `EntityLocation`
- Moreover it's useless for `UnsafeWorldCell::fetch_table` to return the `TableRow` too, since the caller must already have access to the `EntityLocation` which contains the `TableRow`.
- The result is that `UnsafeWorldCell::fetch_table` now only does 2 memory fetches instead of 4.

## Solution

- Revert the changes to the implementation of `UnsafeWorldCell::fetch_table` made in bevyengine#6402
  • Loading branch information
SkiFire13 authored and myreprise1 committed Feb 15, 2023
1 parent 41a8e37 commit ddcdf0f
Showing 1 changed file with 11 additions and 15 deletions.
26 changes: 11 additions & 15 deletions crates/bevy_ecs/src/world/unsafe_world_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
},
entity::{Entities, Entity, EntityLocation},
prelude::Component,
storage::{Column, ComponentSparseSet, TableRow},
storage::{Column, ComponentSparseSet},
system::Resource,
};
use bevy_ptr::Ptr;
Expand Down Expand Up @@ -754,14 +754,10 @@ impl<'w> UnsafeWorldCell<'w> {
self,
location: EntityLocation,
component_id: ComponentId,
) -> Option<(&'w Column, TableRow)> {
let archetype = &self.archetypes()[location.archetype_id];
) -> Option<&'w Column> {
// SAFETY: caller ensures returned data is not misused and we have not created any borrows
// of component/resource data
let table = &unsafe { self.unsafe_world() }.storages.tables[archetype.table_id()];
let components = table.get_column(component_id)?;
let table_row = archetype.entity_table_row(location.archetype_row);
Some((components, table_row))
unsafe { self.unsafe_world() }.storages.tables[location.table_id].get_column(component_id)
}

#[inline]
Expand Down Expand Up @@ -799,9 +795,9 @@ unsafe fn get_component(
// SAFETY: component_id exists and is therefore valid
match storage_type {
StorageType::Table => {
let (components, table_row) = world.fetch_table(location, component_id)?;
let components = world.fetch_table(location, component_id)?;
// SAFETY: archetypes only store valid table_rows and caller ensure aliasing rules
Some(components.get_data_unchecked(table_row))
Some(components.get_data_unchecked(location.table_row))
}
StorageType::SparseSet => world.fetch_sparse_set(component_id)?.get(entity),
}
Expand All @@ -825,14 +821,14 @@ unsafe fn get_component_and_ticks(
) -> Option<(Ptr<'_>, TickCells<'_>)> {
match storage_type {
StorageType::Table => {
let (components, table_row) = world.fetch_table(location, component_id)?;
let components = world.fetch_table(location, component_id)?;

// SAFETY: archetypes only store valid table_rows and caller ensure aliasing rules
Some((
components.get_data_unchecked(table_row),
components.get_data_unchecked(location.table_row),
TickCells {
added: components.get_added_ticks_unchecked(table_row),
changed: components.get_changed_ticks_unchecked(table_row),
added: components.get_added_ticks_unchecked(location.table_row),
changed: components.get_changed_ticks_unchecked(location.table_row),
},
))
}
Expand All @@ -859,9 +855,9 @@ unsafe fn get_ticks(
) -> Option<ComponentTicks> {
match storage_type {
StorageType::Table => {
let (components, table_row) = world.fetch_table(location, component_id)?;
let components = world.fetch_table(location, component_id)?;
// SAFETY: archetypes only store valid table_rows and caller ensure aliasing rules
Some(components.get_ticks_unchecked(table_row))
Some(components.get_ticks_unchecked(location.table_row))
}
StorageType::SparseSet => world.fetch_sparse_set(component_id)?.get_ticks(entity),
}
Expand Down

0 comments on commit ddcdf0f

Please sign in to comment.