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

Migrate the rest of the engine to UnsafeWorldCell #8833

Merged
merged 23 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from 22 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
3 changes: 2 additions & 1 deletion benches/benches/bevy_ecs/world/world_get.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,10 @@ pub fn query_get_component_simple(criterion: &mut Criterion) {
let entity = world.spawn(A(0.0)).id();
let mut query = world.query::<&mut A>();

let world_cell = world.as_unsafe_world_cell();
bencher.iter(|| {
for _x in 0..100000 {
let mut a = unsafe { query.get_unchecked(&world, entity).unwrap() };
let mut a = unsafe { query.get_unchecked(world_cell, entity).unwrap() };
a.0 += 1.0;
}
});
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/macros/src/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ pub fn derive_world_query_impl(input: TokenStream) -> TokenStream {
}

unsafe fn init_fetch<'__w>(
_world: &'__w #path::world::World,
_world: #path::world::unsafe_world_cell::UnsafeWorldCell<'__w>,
state: &Self::State,
_last_run: #path::component::Tick,
_this_run: #path::component::Tick,
Expand Down
54 changes: 37 additions & 17 deletions crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
entity::Entity,
query::{Access, DebugCheckedUnwrap, FilteredAccess},
storage::{ComponentSparseSet, Table, TableRow},
world::{Mut, Ref, World},
world::{unsafe_world_cell::UnsafeWorldCell, Mut, Ref, World},
};
pub use bevy_ecs_macros::WorldQuery;
use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref};
Expand Down Expand Up @@ -335,10 +335,11 @@ pub unsafe trait WorldQuery {
///
/// # Safety
///
/// `state` must have been initialized (via [`WorldQuery::init_state`]) using the same `world` passed
/// in to this function.
/// - `world` must have permission to access any of the components specified in `Self::update_archetype_component_access`.
/// - `state` must have been initialized (via [`WorldQuery::init_state`]) using the same `world` passed
/// in to this function.
unsafe fn init_fetch<'w>(
world: &'w World,
world: UnsafeWorldCell<'w>,
state: &Self::State,
last_run: Tick,
this_run: Tick,
Expand Down Expand Up @@ -372,8 +373,10 @@ pub unsafe trait WorldQuery {
///
/// # Safety
///
/// `archetype` and `tables` must be from the [`World`] [`WorldQuery::init_state`] was called on. `state` must
/// be the [`Self::State`] this was initialized with.
/// - `archetype` and `tables` must be from the same [`World`] that [`WorldQuery::init_state`] was called on.
/// - [`Self::update_archetype_component_access`] must have been previously called with `archetype`.
/// - `table` must correspond to `archetype`.
/// - `state` must be the [`State`](Self::State) that `fetch` was initialized with.
unsafe fn set_archetype<'w>(
fetch: &mut Self::Fetch<'w>,
state: &Self::State,
Expand All @@ -386,8 +389,10 @@ pub unsafe trait WorldQuery {
///
/// # Safety
///
/// `table` must be from the [`World`] [`WorldQuery::init_state`] was called on. `state` must be the
/// [`Self::State`] this was initialized with.
/// - `table` must be from the same [`World`] that [`WorldQuery::init_state`] was called on.
/// - `table` must belong to an archetype that was previously registered with
/// [`Self::update_archetype_component_access`].
/// - `state` must be the [`State`](Self::State) that `fetch` was initialized with.
unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table);

/// Fetch [`Self::Item`](`WorldQuery::Item`) for either the given `entity` in the current [`Table`],
Expand Down Expand Up @@ -475,7 +480,7 @@ unsafe impl WorldQuery for Entity {
const IS_ARCHETYPAL: bool = true;

unsafe fn init_fetch<'w>(
_world: &'w World,
_world: UnsafeWorldCell<'w>,
_state: &Self::State,
_last_run: Tick,
_this_run: Tick,
Expand Down Expand Up @@ -558,7 +563,7 @@ unsafe impl<T: Component> WorldQuery for &T {

#[inline]
unsafe fn init_fetch<'w>(
world: &'w World,
world: UnsafeWorldCell<'w>,
&component_id: &ComponentId,
_last_run: Tick,
_this_run: Tick,
Expand All @@ -567,6 +572,11 @@ unsafe impl<T: Component> WorldQuery for &T {
table_components: None,
sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| {
world
// SAFETY: The underlying type associated with `component_id` is `T`,
// which we are allowed to access since we registered it in `update_archetype_component_access`.
// Note that we do not actually access any components in this function, we just get a shared
// reference to the sparse set, which is used to access the components in `Self::fetch`.
.unsafe_world()
.storages()
.sparse_sets
.get(component_id)
Expand Down Expand Up @@ -704,7 +714,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> {

#[inline]
unsafe fn init_fetch<'w>(
world: &'w World,
world: UnsafeWorldCell<'w>,
&component_id: &ComponentId,
last_run: Tick,
this_run: Tick,
Expand All @@ -713,6 +723,8 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> {
table_data: None,
sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| {
world
// SAFETY: See &T::init_fetch.
.unsafe_world()
.storages()
.sparse_sets
.get(component_id)
Expand Down Expand Up @@ -866,7 +878,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {

#[inline]
unsafe fn init_fetch<'w>(
world: &'w World,
world: UnsafeWorldCell<'w>,
&component_id: &ComponentId,
last_run: Tick,
this_run: Tick,
Expand All @@ -875,6 +887,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
table_data: None,
sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| {
world
// SAFETY: See &T::init_fetch.
.unsafe_world()
.storages()
.sparse_sets
.get(component_id)
Expand Down Expand Up @@ -1011,7 +1025,7 @@ unsafe impl<T: WorldQuery> WorldQuery for Option<T> {

#[inline]
unsafe fn init_fetch<'w>(
world: &'w World,
world: UnsafeWorldCell<'w>,
state: &T::State,
last_run: Tick,
this_run: Tick,
Expand Down Expand Up @@ -1116,7 +1130,7 @@ macro_rules! impl_tuple_fetch {

#[inline]
#[allow(clippy::unused_unit)]
unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> {
unsafe fn init_fetch<'w>(_world: UnsafeWorldCell<'w>, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> {
let ($($name,)*) = state;
($($name::init_fetch(_world, $name, _last_run, _this_run),)*)
}
Expand Down Expand Up @@ -1226,7 +1240,7 @@ macro_rules! impl_anytuple_fetch {

#[inline]
#[allow(clippy::unused_unit)]
unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> {
unsafe fn init_fetch<'w>(_world: UnsafeWorldCell<'w>, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> {
let ($($name,)*) = state;
($(($name::init_fetch(_world, $name, _last_run, _this_run), false),)*)
}
Expand Down Expand Up @@ -1350,7 +1364,13 @@ unsafe impl<Q: WorldQuery> WorldQuery for NopWorldQuery<Q> {
const IS_ARCHETYPAL: bool = true;

#[inline(always)]
unsafe fn init_fetch(_world: &World, _state: &Q::State, _last_run: Tick, _this_run: Tick) {}
unsafe fn init_fetch(
_world: UnsafeWorldCell,
_state: &Q::State,
_last_run: Tick,
_this_run: Tick,
) {
}

unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {}

Expand Down Expand Up @@ -1408,7 +1428,7 @@ unsafe impl<T: ?Sized> WorldQuery for PhantomData<T> {
fn shrink<'wlong: 'wshort, 'wshort>(_item: Self::Item<'wlong>) -> Self::Item<'wshort> {}

unsafe fn init_fetch<'w>(
_world: &'w World,
_world: UnsafeWorldCell<'w>,
_state: &Self::State,
_last_run: Tick,
_this_run: Tick,
Expand Down
30 changes: 24 additions & 6 deletions crates/bevy_ecs/src/query/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
entity::Entity,
query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery},
storage::{Column, ComponentSparseSet, Table, TableRow},
world::World,
world::{unsafe_world_cell::UnsafeWorldCell, World},
};
use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref};
use bevy_utils::all_tuples;
Expand Down Expand Up @@ -51,7 +51,13 @@ unsafe impl<T: Component> WorldQuery for With<T> {
fn shrink<'wlong: 'wshort, 'wshort>(_: Self::Item<'wlong>) -> Self::Item<'wshort> {}

#[inline]
unsafe fn init_fetch(_world: &World, _state: &ComponentId, _last_run: Tick, _this_run: Tick) {}
unsafe fn init_fetch(
_world: UnsafeWorldCell,
_state: &ComponentId,
_last_run: Tick,
_this_run: Tick,
) {
}

unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {}

Expand Down Expand Up @@ -148,7 +154,13 @@ unsafe impl<T: Component> WorldQuery for Without<T> {
fn shrink<'wlong: 'wshort, 'wshort>(_: Self::Item<'wlong>) -> Self::Item<'wshort> {}

#[inline]
unsafe fn init_fetch(_world: &World, _state: &ComponentId, _last_run: Tick, _this_run: Tick) {}
unsafe fn init_fetch(
_world: UnsafeWorldCell,
_state: &ComponentId,
_last_run: Tick,
_this_run: Tick,
) {
}

unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {}

Expand Down Expand Up @@ -268,7 +280,7 @@ macro_rules! impl_query_filter_tuple {
const IS_ARCHETYPAL: bool = true $(&& $filter::IS_ARCHETYPAL)*;

#[inline]
unsafe fn init_fetch<'w>(world: &'w World, state: &Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> {
unsafe fn init_fetch<'w>(world: UnsafeWorldCell<'w>, state: &Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> {
let ($($filter,)*) = state;
($(OrFetch {
fetch: $filter::init_fetch(world, $filter, last_run, this_run),
Expand Down Expand Up @@ -412,12 +424,18 @@ macro_rules! impl_tick_filter {
}

#[inline]
unsafe fn init_fetch<'w>(world: &'w World, &id: &ComponentId, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> {
unsafe fn init_fetch<'w>(
world: UnsafeWorldCell<'w>,
&id: &ComponentId,
last_run: Tick,
this_run: Tick
) -> Self::Fetch<'w> {
Self::Fetch::<'w> {
table_ticks: None,
sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet)
.then(|| {
world.storages()
world.unsafe_world()
.storages()
.sparse_sets
.get(id)
.debug_checked_unwrap()
Expand Down
51 changes: 26 additions & 25 deletions crates/bevy_ecs/src/query/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ use crate::{
archetype::{ArchetypeEntity, ArchetypeId, Archetypes},
component::Tick,
entity::{Entities, Entity},
prelude::World,
query::{ArchetypeFilter, DebugCheckedUnwrap, QueryState, WorldQuery},
storage::{TableId, TableRow, Tables},
world::unsafe_world_cell::UnsafeWorldCell,
};
use std::{borrow::Borrow, iter::FusedIterator, marker::PhantomData, mem::MaybeUninit};

Expand All @@ -23,20 +23,19 @@ pub struct QueryIter<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> {

impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIter<'w, 's, Q, F> {
/// # Safety
/// This does not check for mutable query correctness. To be safe, make sure mutable queries
/// have unique access to the components they query.
/// This does not validate that `world.id()` matches `query_state.world_id`. Calling this on a `world`
/// with a mismatched [`WorldId`](crate::world::WorldId) is unsound.
/// - `world` must have permission to access any of the components registered in `query_state`.
/// - `world` must be the same one used to initialize `query_state`.
pub(crate) unsafe fn new(
world: &'w World,
world: UnsafeWorldCell<'w>,
query_state: &'s QueryState<Q, F>,
last_run: Tick,
this_run: Tick,
) -> Self {
QueryIter {
query_state,
tables: &world.storages().tables,
archetypes: &world.archetypes,
// SAFETY: We only access table data that has been registered in `query_state`.
tables: &world.unsafe_world().storages().tables,
archetypes: world.archetypes(),
cursor: QueryIterationCursor::init(world, query_state, last_run, this_run),
}
}
Expand Down Expand Up @@ -91,12 +90,10 @@ where
I::Item: Borrow<Entity>,
{
/// # Safety
/// This does not check for mutable query correctness. To be safe, make sure mutable queries
/// have unique access to the components they query.
/// This does not validate that `world.id()` matches `query_state.world_id`. Calling this on a `world`
/// with a mismatched [`WorldId`](crate::world::WorldId) is unsound.
/// - `world` must have permission to access any of the components registered in `query_state`.
/// - `world` must be the same one used to initialize `query_state`.
pub(crate) unsafe fn new<EntityList: IntoIterator<IntoIter = I>>(
world: &'w World,
world: UnsafeWorldCell<'w>,
query_state: &'s QueryState<Q, F>,
entity_list: EntityList,
last_run: Tick,
Expand All @@ -106,9 +103,11 @@ where
let filter = F::init_fetch(world, &query_state.filter_state, last_run, this_run);
QueryManyIter {
query_state,
entities: &world.entities,
archetypes: &world.archetypes,
tables: &world.storages.tables,
entities: world.entities(),
archetypes: world.archetypes(),
// SAFETY: We only access table data that has been registered in `query_state`.
// This means `world` has permission to access the data we use.
tables: &world.unsafe_world().storages.tables,
fetch,
filter,
entity_iter: entity_list.into_iter(),
Expand Down Expand Up @@ -282,12 +281,10 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, const K: usize>
QueryCombinationIter<'w, 's, Q, F, K>
{
/// # Safety
/// This does not check for mutable query correctness. To be safe, make sure mutable queries
/// have unique access to the components they query.
/// This does not validate that `world.id()` matches `query_state.world_id`. Calling this on a
/// `world` with a mismatched [`WorldId`](crate::world::WorldId) is unsound.
/// - `world` must have permission to access any of the components registered in `query_state`.
/// - `world` must be the same one used to initialize `query_state`.
pub(crate) unsafe fn new(
world: &'w World,
world: UnsafeWorldCell<'w>,
query_state: &'s QueryState<Q, F>,
last_run: Tick,
this_run: Tick,
Expand Down Expand Up @@ -318,8 +315,9 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery, const K: usize>

QueryCombinationIter {
query_state,
tables: &world.storages().tables,
archetypes: &world.archetypes,
// SAFETY: We only access table data that has been registered in `query_state`.
tables: &world.unsafe_world().storages().tables,
archetypes: world.archetypes(),
cursors: array.assume_init(),
}
}
Expand Down Expand Up @@ -485,7 +483,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's,
const IS_DENSE: bool = Q::IS_DENSE && F::IS_DENSE;

unsafe fn init_empty(
world: &'w World,
world: UnsafeWorldCell<'w>,
query_state: &'s QueryState<Q, F>,
last_run: Tick,
this_run: Tick,
Expand All @@ -497,8 +495,11 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's,
}
}

/// # Safety
/// - `world` must have permission to access any of the components registered in `query_state`.
/// - `world` must be the same one used to initialize `query_state`.
unsafe fn init(
world: &'w World,
world: UnsafeWorldCell<'w>,
query_state: &'s QueryState<Q, F>,
last_run: Tick,
this_run: Tick,
Expand Down
Loading