Skip to content

Stop storing access for all systems #19477

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
merged 12 commits into from
Jun 13, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
12 changes: 8 additions & 4 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,10 +440,10 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
> #path::system::SystemParamBuilder<#generic_struct> for #builder_name<#(#builder_type_parameters,)*>
#where_clause
{
fn build(self, world: &mut #path::world::World, meta: &mut #path::system::SystemMeta) -> <#generic_struct as #path::system::SystemParam>::State {
fn build(self, world: &mut #path::world::World) -> <#generic_struct as #path::system::SystemParam>::State {
let #builder_name { #(#fields: #field_locals,)* } = self;
#state_struct_name {
state: #path::system::SystemParamBuilder::build((#(#tuple_patterns,)*), world, meta)
state: #path::system::SystemParamBuilder::build((#(#tuple_patterns,)*), world)
}
}
}
Expand Down Expand Up @@ -472,12 +472,16 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
type State = #state_struct_name<#punctuated_generic_idents>;
type Item<'w, 's> = #struct_name #ty_generics;

fn init_state(world: &mut #path::world::World, system_meta: &mut #path::system::SystemMeta) -> Self::State {
fn init_state(world: &mut #path::world::World) -> Self::State {
#state_struct_name {
state: <#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::init_state(world, system_meta),
state: <#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::init_state(world),
}
}

fn init_access(state: &Self::State, system_meta: &mut #path::system::SystemMeta, component_access_set: &mut #path::query::FilteredAccessSet<#path::component::ComponentId>, world: &mut #path::world::World) {
<#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::init_access(&state.state, system_meta, component_access_set, world);
}

fn apply(state: &mut Self::State, system_meta: &#path::system::SystemMeta, world: &mut #path::world::World) {
<#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::apply(&mut state.state, system_meta, world);
}
Expand Down
11 changes: 10 additions & 1 deletion crates/bevy_ecs/src/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ use crate::{
component::{Component, ComponentId, ComponentIdFor, Tick},
entity::Entity,
event::{Event, EventCursor, EventId, EventIterator, EventIteratorWithId, Events},
query::FilteredAccessSet,
relationship::RelationshipHookMode,
storage::SparseSet,
system::{Local, ReadOnlySystemParam, SystemMeta, SystemParam},
Expand Down Expand Up @@ -617,7 +618,15 @@ unsafe impl<'a> SystemParam for &'a RemovedComponentEvents {
type State = ();
type Item<'w, 's> = &'w RemovedComponentEvents;

fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {}
fn init_state(_world: &mut World) -> Self::State {}

fn init_access(
_state: &Self::State,
_system_meta: &mut SystemMeta,
_component_access_set: &mut FilteredAccessSet<ComponentId>,
_world: &mut World,
) {
}

#[inline]
unsafe fn get_param<'w, 's>(
Expand Down
16 changes: 12 additions & 4 deletions crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
let mut system_has_conditions_cache = HashMap::<usize, bool>::default();
let mut is_valid_explicit_sync_point = |system: NodeId| {
let index = system.index();
is_apply_deferred(graph.systems[index].get().unwrap())
is_apply_deferred(&graph.systems[index].get().unwrap().system)
&& !*system_has_conditions_cache
.entry(index)
.or_insert_with(|| system_has_conditions(graph, system))
Expand Down Expand Up @@ -138,7 +138,11 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
} else if !node_needs_sync {
// No previous node has postponed sync points to add so check if the system itself
// has deferred params that require a sync point to apply them.
node_needs_sync = graph.systems[node.index()].get().unwrap().has_deferred();
node_needs_sync = graph.systems[node.index()]
.get()
.unwrap()
.system
.has_deferred();
}

for target in dependency_flattened.neighbors_directed(*node, Direction::Outgoing) {
Expand All @@ -148,7 +152,11 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {

let mut edge_needs_sync = node_needs_sync;
if node_needs_sync
&& !graph.systems[target.index()].get().unwrap().is_exclusive()
&& !graph.systems[target.index()]
.get()
.unwrap()
.system
.is_exclusive()
&& self.no_sync_edges.contains(&(*node, target))
{
// The node has deferred params to apply, but this edge is ignoring sync points.
Expand Down Expand Up @@ -190,7 +198,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
continue;
}

if is_apply_deferred(graph.systems[target.index()].get().unwrap()) {
if is_apply_deferred(&graph.systems[target.index()].get().unwrap().system) {
// We don't need to insert a sync point since ApplyDeferred is a sync point
// already!
continue;
Expand Down
17 changes: 7 additions & 10 deletions crates/bevy_ecs/src/schedule/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
error::{BevyError, ErrorContext, Result},
prelude::{IntoSystemSet, SystemSet},
query::FilteredAccessSet,
schedule::{BoxedCondition, InternedSystemSet, NodeId, SystemTypeSet},
schedule::{ConditionWithAccess, InternedSystemSet, NodeId, SystemTypeSet, SystemWithAccess},
system::{ScheduleSystem, System, SystemIn, SystemParamValidationError, SystemStateFlags},
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
};
Expand Down Expand Up @@ -74,9 +74,9 @@ pub struct SystemSchedule {
/// List of system node ids.
pub(super) system_ids: Vec<NodeId>,
/// Indexed by system node id.
pub(super) systems: Vec<ScheduleSystem>,
pub(super) systems: Vec<SystemWithAccess>,
/// Indexed by system node id.
pub(super) system_conditions: Vec<Vec<BoxedCondition>>,
pub(super) system_conditions: Vec<Vec<ConditionWithAccess>>,
/// Indexed by system node id.
/// Number of systems that the system immediately depends on.
#[cfg_attr(
Expand All @@ -97,7 +97,7 @@ pub struct SystemSchedule {
/// List of system set node ids.
pub(super) set_ids: Vec<NodeId>,
/// Indexed by system set node id.
pub(super) set_conditions: Vec<Vec<BoxedCondition>>,
pub(super) set_conditions: Vec<Vec<ConditionWithAccess>>,
/// Indexed by system set node id.
/// List of systems that are in sets that have conditions.
///
Expand Down Expand Up @@ -162,11 +162,6 @@ impl System for ApplyDeferred {
Cow::Borrowed("bevy_ecs::apply_deferred")
}

fn component_access_set(&self) -> &FilteredAccessSet<ComponentId> {
// This system accesses no components.
const { &FilteredAccessSet::new() }
}

fn flags(&self) -> SystemStateFlags {
// non-send , exclusive , no deferred
SystemStateFlags::NON_SEND | SystemStateFlags::EXCLUSIVE
Expand Down Expand Up @@ -205,7 +200,9 @@ impl System for ApplyDeferred {
Ok(())
}

fn initialize(&mut self, _world: &mut World) {}
fn initialize(&mut self, _world: &mut World) -> FilteredAccessSet<ComponentId> {
FilteredAccessSet::new()
}

fn check_change_tick(&mut self, _change_tick: Tick) {}

Expand Down
53 changes: 26 additions & 27 deletions crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ use tracing::{info_span, Span};
use crate::{
error::{ErrorContext, ErrorHandler, Result},
prelude::Resource,
schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule},
schedule::{
is_apply_deferred, ConditionWithAccess, ExecutorKind, SystemExecutor, SystemSchedule,
SystemWithAccess,
},
system::ScheduleSystem,
world::{unsafe_world_cell::UnsafeWorldCell, World},
};
Expand All @@ -27,14 +30,14 @@ use super::__rust_begin_short_backtrace;
/// Borrowed data used by the [`MultiThreadedExecutor`].
struct Environment<'env, 'sys> {
executor: &'env MultiThreadedExecutor,
systems: &'sys [SyncUnsafeCell<ScheduleSystem>],
systems: &'sys [SyncUnsafeCell<SystemWithAccess>],
conditions: SyncUnsafeCell<Conditions<'sys>>,
world_cell: UnsafeWorldCell<'env>,
}

struct Conditions<'a> {
system_conditions: &'a mut [Vec<BoxedCondition>],
set_conditions: &'a mut [Vec<BoxedCondition>],
system_conditions: &'a mut [Vec<ConditionWithAccess>],
set_conditions: &'a mut [Vec<ConditionWithAccess>],
sets_with_conditions_of_systems: &'a [FixedBitSet],
systems_in_sets_with_conditions: &'a [FixedBitSet],
}
Expand Down Expand Up @@ -172,8 +175,8 @@ impl SystemExecutor for MultiThreadedExecutor {
conflicting_systems: FixedBitSet::with_capacity(sys_count),
condition_conflicting_systems: FixedBitSet::with_capacity(sys_count),
dependents: schedule.system_dependents[index].clone(),
is_send: schedule.systems[index].is_send(),
is_exclusive: schedule.systems[index].is_exclusive(),
is_send: schedule.systems[index].system.is_send(),
is_exclusive: schedule.systems[index].system.is_exclusive(),
});
if schedule.system_dependencies[index] == 0 {
self.starting_systems.insert(index);
Expand All @@ -187,10 +190,7 @@ impl SystemExecutor for MultiThreadedExecutor {
let system1 = &schedule.systems[index1];
for index2 in 0..index1 {
let system2 = &schedule.systems[index2];
if !system2
.component_access_set()
.is_compatible(system1.component_access_set())
{
if !system2.access.is_compatible(&system1.access) {
state.system_task_metadata[index1]
.conflicting_systems
.insert(index2);
Expand All @@ -202,11 +202,10 @@ impl SystemExecutor for MultiThreadedExecutor {

for index2 in 0..sys_count {
let system2 = &schedule.systems[index2];
if schedule.system_conditions[index1].iter().any(|condition| {
!system2
.component_access_set()
.is_compatible(condition.component_access_set())
}) {
if schedule.system_conditions[index1]
.iter()
.any(|condition| !system2.access.is_compatible(&condition.access))
{
state.system_task_metadata[index1]
.condition_conflicting_systems
.insert(index2);
Expand All @@ -220,11 +219,10 @@ impl SystemExecutor for MultiThreadedExecutor {
let mut conflicting_systems = FixedBitSet::with_capacity(sys_count);
for sys_index in 0..sys_count {
let system = &schedule.systems[sys_index];
if schedule.set_conditions[set_idx].iter().any(|condition| {
!system
.component_access_set()
.is_compatible(condition.component_access_set())
}) {
if schedule.set_conditions[set_idx]
.iter()
.any(|condition| !system.access.is_compatible(&condition.access))
{
conflicting_systems.insert(sys_index);
}
}
Expand Down Expand Up @@ -468,7 +466,8 @@ impl ExecutorState {
debug_assert!(!self.running_systems.contains(system_index));
// SAFETY: Caller assured that these systems are not running.
// Therefore, no other reference to this system exists and there is no aliasing.
let system = unsafe { &mut *context.environment.systems[system_index].get() };
let system =
&mut unsafe { &mut *context.environment.systems[system_index].get() }.system;

#[cfg(feature = "hotpatching")]
if should_update_hotpatch {
Expand Down Expand Up @@ -661,7 +660,7 @@ impl ExecutorState {
/// used by the specified system.
unsafe fn spawn_system_task(&mut self, context: &Context, system_index: usize) {
// SAFETY: this system is not running, no other reference exists
let system = unsafe { &mut *context.environment.systems[system_index].get() };
let system = &mut unsafe { &mut *context.environment.systems[system_index].get() }.system;
// Move the full context object into the new future.
let context = *context;

Expand Down Expand Up @@ -703,7 +702,7 @@ impl ExecutorState {
/// Caller must ensure no systems are currently borrowed.
unsafe fn spawn_exclusive_system_task(&mut self, context: &Context, system_index: usize) {
// SAFETY: this system is not running, no other reference exists
let system = unsafe { &mut *context.environment.systems[system_index].get() };
let system = &mut unsafe { &mut *context.environment.systems[system_index].get() }.system;
// Move the full context object into the new future.
let context = *context;

Expand Down Expand Up @@ -785,12 +784,12 @@ impl ExecutorState {

fn apply_deferred(
unapplied_systems: &FixedBitSet,
systems: &[SyncUnsafeCell<ScheduleSystem>],
systems: &[SyncUnsafeCell<SystemWithAccess>],
world: &mut World,
) -> Result<(), Box<dyn Any + Send>> {
for system_index in unapplied_systems.ones() {
// SAFETY: none of these systems are running, no other references exist
let system = unsafe { &mut *systems[system_index].get() };
let system = &mut unsafe { &mut *systems[system_index].get() }.system;
let res = std::panic::catch_unwind(AssertUnwindSafe(|| {
system.apply_deferred(world);
}));
Expand All @@ -813,7 +812,7 @@ fn apply_deferred(
/// - `world` must have permission to read any world data
/// required by `conditions`.
unsafe fn evaluate_and_fold_conditions(
conditions: &mut [BoxedCondition],
conditions: &mut [ConditionWithAccess],
world: UnsafeWorldCell,
error_handler: ErrorHandler,
) -> bool {
Expand All @@ -823,7 +822,7 @@ unsafe fn evaluate_and_fold_conditions(
)]
conditions
.iter_mut()
.map(|condition| {
.map(|ConditionWithAccess { condition, .. }| {
// SAFETY:
// - The caller ensures that `world` has permission to read any data
// required by the condition.
Expand Down
11 changes: 6 additions & 5 deletions crates/bevy_ecs/src/schedule/executor/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ use std::eprintln;
use crate::{
error::{ErrorContext, ErrorHandler},
schedule::{
executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule,
executor::is_apply_deferred, ConditionWithAccess, ExecutorKind, SystemExecutor,
SystemSchedule,
},
world::World,
};
Expand Down Expand Up @@ -70,7 +71,7 @@ impl SystemExecutor for SimpleExecutor {

for system_index in 0..schedule.systems.len() {
#[cfg(feature = "trace")]
let name = schedule.systems[system_index].name();
let name = schedule.systems[system_index].system.name();
#[cfg(feature = "trace")]
let should_run_span = info_span!("check_conditions", name = &*name).entered();

Expand Down Expand Up @@ -105,7 +106,7 @@ impl SystemExecutor for SimpleExecutor {

should_run &= system_conditions_met;

let system = &mut schedule.systems[system_index];
let system = &mut schedule.systems[system_index].system;
if should_run {
let valid_params = match system.validate_param(world) {
Ok(()) => true,
Expand Down Expand Up @@ -195,7 +196,7 @@ impl SimpleExecutor {
note = "Use SingleThreadedExecutor instead. See https://github.com/bevyengine/bevy/issues/18453 for motivation."
)]
fn evaluate_and_fold_conditions(
conditions: &mut [BoxedCondition],
conditions: &mut [ConditionWithAccess],
world: &mut World,
error_handler: ErrorHandler,
) -> bool {
Expand All @@ -211,7 +212,7 @@ fn evaluate_and_fold_conditions(
)]
conditions
.iter_mut()
.map(|condition| {
.map(|ConditionWithAccess { condition, .. }| {
match condition.validate_param(world) {
Ok(()) => (),
Err(e) => {
Expand Down
Loading