Skip to content

Commit 2e1e7f1

Browse files
maniwanihymm
andcommitted
rebase
- add `MaybeUnsafeCell` (in the future `&T -> &UnsafeCell<T>` may be OK) - add `WorldAccessLevel` - add concept of "all exclusive access" to `Access` - update system_param.rs - update system.rs - update function_system.rs - update system_chaining.rs - remove ExclusiveSystem* internals - add tests - fix stage tests - in ambiguity_detection, the else branch in find_ambiguities is no longer taken because exclusive systems have a component access now. And obv two empty systems will always be compatible. - update fixed_timestep.rs - update extract_param.rs Co-Authored-By: Mike <mike.hsu@gmail.com>
1 parent 07d5769 commit 2e1e7f1

File tree

37 files changed

+991
-956
lines changed

37 files changed

+991
-956
lines changed

benches/benches/bevy_ecs/scheduling/run_criteria.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use bevy_ecs::{
22
component::Component,
3-
prelude::{ParallelSystemDescriptorCoercion, Res, RunCriteriaDescriptorCoercion},
3+
prelude::{IntoSystemDescriptor, Res, RunCriteriaDescriptorCoercion},
44
schedule::{ShouldRun, Stage, SystemStage},
55
system::Query,
66
world::World,

crates/bevy_animation/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use bevy_ecs::{
1212
entity::Entity,
1313
prelude::Component,
1414
reflect::ReflectComponent,
15-
schedule::ParallelSystemDescriptorCoercion,
15+
schedule::IntoSystemDescriptor,
1616
system::{Query, Res},
1717
};
1818
use bevy_hierarchy::Children;

crates/bevy_app/src/app.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use crate::{CoreStage, Plugin, PluginGroup, PluginGroupBuilder, StartupSchedule,
22
pub use bevy_derive::AppLabel;
33
use bevy_ecs::{
44
event::{Event, Events},
5-
prelude::{FromWorld, IntoExclusiveSystem},
5+
prelude::FromWorld,
66
schedule::{
77
IntoSystemDescriptor, Schedule, ShouldRun, Stage, StageLabel, State, StateData, SystemSet,
88
SystemStage,
@@ -78,7 +78,7 @@ impl Default for App {
7878

7979
app.add_default_stages()
8080
.add_event::<AppExit>()
81-
.add_system_to_stage(CoreStage::Last, World::clear_trackers.exclusive_system());
81+
.add_system_to_stage(CoreStage::Last, World::clear_trackers.at_end());
8282

8383
#[cfg(feature = "bevy_ci_testing")]
8484
{

crates/bevy_ecs/macros/src/lib.rs

+30-2
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,30 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream {
254254
fn apply(&mut self, world: &mut World) {
255255
self.0.apply(world)
256256
}
257+
258+
fn world_access_level() -> WorldAccessLevel {
259+
let mut exclusive = false;
260+
let mut shared = false;
261+
#(
262+
match #param_fetch::world_access_level() {
263+
WorldAccessLevel::Exclusive => {
264+
exclusive = true;
265+
}
266+
WorldAccessLevel::Shared => {
267+
shared = true;
268+
}
269+
WorldAccessLevel::None => (),
270+
}
271+
)*
272+
273+
if exclusive {
274+
WorldAccessLevel::Exclusive
275+
} else if shared {
276+
WorldAccessLevel::Shared
277+
} else {
278+
WorldAccessLevel::None
279+
}
280+
}
257281
}
258282

259283

@@ -266,7 +290,7 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream {
266290
unsafe fn get_param(
267291
state: &'s mut Self,
268292
system_meta: &SystemMeta,
269-
world: &'w World,
293+
world: MaybeUnsafeCell<'w, World>,
270294
change_tick: u32,
271295
) -> Self::Item {
272296
ParamSet {
@@ -404,14 +428,18 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
404428
fn apply(&mut self, world: &mut #path::world::World) {
405429
self.state.apply(world)
406430
}
431+
432+
fn world_access_level() -> #path::system::WorldAccessLevel {
433+
TSystemParamState::world_access_level()
434+
}
407435
}
408436

409437
impl #impl_generics #path::system::SystemParamFetch<'w, 's> for FetchState <(#(<#field_types as #path::system::SystemParam>::Fetch,)*), #punctuated_generic_idents> #where_clause {
410438
type Item = #struct_name #ty_generics;
411439
unsafe fn get_param(
412440
state: &'s mut Self,
413441
system_meta: &#path::system::SystemMeta,
414-
world: &'w #path::world::World,
442+
world: #path::system::MaybeUnsafeCell<'w, #path::world::World>,
415443
change_tick: u32,
416444
) -> Self::Item {
417445
#struct_name {

crates/bevy_ecs/src/entity/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ use std::{
7272
/// }
7373
/// #
7474
/// # bevy_ecs::system::assert_is_system(setup);
75-
/// # bevy_ecs::system::IntoExclusiveSystem::exclusive_system(exclusive_system);
75+
/// # bevy_ecs::system::assert_is_system(exclusive_system);
7676
/// ```
7777
///
7878
/// It can be used to refer to a specific entity to apply [`EntityCommands`], or to call [`Query::get`] (or similar methods) to access its components.

crates/bevy_ecs/src/lib.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,13 @@ pub mod prelude {
3434
event::{EventReader, EventWriter, Events},
3535
query::{Added, AnyOf, ChangeTrackers, Changed, Or, QueryState, With, Without},
3636
schedule::{
37-
AmbiguitySetLabel, ExclusiveSystemDescriptorCoercion, ParallelSystemDescriptorCoercion,
38-
RunCriteria, RunCriteriaDescriptorCoercion, RunCriteriaLabel, Schedule, Stage,
39-
StageLabel, State, SystemLabel, SystemSet, SystemStage,
37+
AmbiguitySetLabel, IntoSystemDescriptor, RunCriteria, RunCriteriaDescriptorCoercion,
38+
RunCriteriaLabel, Schedule, Stage, StageLabel, State, SystemLabel, SystemSet,
39+
SystemStage,
4040
},
4141
system::{
42-
Commands, In, IntoChainSystem, IntoExclusiveSystem, IntoSystem, Local, NonSend,
43-
NonSendMut, ParallelCommands, ParamSet, Query, RemovedComponents, Res, ResMut, System,
42+
Commands, In, IntoChainSystem, IntoSystem, Local, NonSend, NonSendMut,
43+
ParallelCommands, ParamSet, Query, RemovedComponents, Res, ResMut, System,
4444
SystemParamFunction,
4545
},
4646
world::{FromWorld, Mut, World},

crates/bevy_ecs/src/query/access.rs

+71-9
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,20 @@ pub struct Access<T: SparseSetIndex> {
1313
reads_and_writes: FixedBitSet,
1414
/// The exclusively-accessed elements.
1515
writes: FixedBitSet,
16-
/// Is `true` if this has access to all elements in the collection?
16+
/// Is `true` if this has access to all elements in the collection.
1717
/// This field is a performance optimization for `&World` (also harder to mess up for soundness).
1818
reads_all: bool,
19+
/// Is `true` if this has exclusive access to all elements in the collection.
20+
/// This field is a performance optimization for `&mut World` (also harder to mess up for soundness).
21+
writes_all: bool,
1922
marker: PhantomData<T>,
2023
}
2124

2225
impl<T: SparseSetIndex> Default for Access<T> {
2326
fn default() -> Self {
2427
Self {
2528
reads_all: false,
29+
writes_all: false,
2630
reads_and_writes: Default::default(),
2731
writes: Default::default(),
2832
marker: PhantomData,
@@ -64,7 +68,11 @@ impl<T: SparseSetIndex> Access<T> {
6468

6569
/// Returns `true` if this can exclusively access the element given by `index`.
6670
pub fn has_write(&self, index: T) -> bool {
67-
self.writes.contains(index.sparse_set_index())
71+
if self.writes_all {
72+
true
73+
} else {
74+
self.writes.contains(index.sparse_set_index())
75+
}
6876
}
6977

7078
/// Sets this as having access to all indexed elements (i.e. `&World`).
@@ -77,16 +85,29 @@ impl<T: SparseSetIndex> Access<T> {
7785
self.reads_all
7886
}
7987

88+
/// Sets this as having exclusive access to all indexed elements (i.e. `&mut World`).
89+
pub fn write_all(&mut self) {
90+
self.reads_all = true;
91+
self.writes_all = true;
92+
}
93+
94+
/// Returns `true` if this has exclusive access to all indexed elements (i.e. `&mut World`).
95+
pub fn has_write_all(&self) -> bool {
96+
self.writes_all
97+
}
98+
8099
/// Removes all accesses.
81100
pub fn clear(&mut self) {
82101
self.reads_all = false;
102+
self.writes_all = false;
83103
self.reads_and_writes.clear();
84104
self.writes.clear();
85105
}
86106

87107
/// Adds all access from `other`.
88108
pub fn extend(&mut self, other: &Access<T>) {
89109
self.reads_all = self.reads_all || other.reads_all;
110+
self.writes_all = self.writes_all || other.writes_all;
90111
self.reads_and_writes.union_with(&other.reads_and_writes);
91112
self.writes.union_with(&other.writes);
92113
}
@@ -96,6 +117,12 @@ impl<T: SparseSetIndex> Access<T> {
96117
/// `Access` instances are incompatible if one can write
97118
/// an element that the other can read or write.
98119
pub fn is_compatible(&self, other: &Access<T>) -> bool {
120+
// All systems make a `&World` reference before running to update change detection info.
121+
// Since exclusive systems produce a `&mut World`, we cannot let other systems run.
122+
if self.writes_all || other.writes_all {
123+
return false;
124+
}
125+
99126
// Only systems that do not write data are compatible with systems that operate on `&World`.
100127
if self.reads_all {
101128
return other.writes.count_ones(..) == 0;
@@ -112,15 +139,31 @@ impl<T: SparseSetIndex> Access<T> {
112139
/// Returns a vector of elements that the access and `other` cannot access at the same time.
113140
pub fn get_conflicts(&self, other: &Access<T>) -> Vec<T> {
114141
let mut conflicts = FixedBitSet::default();
115-
if self.reads_all {
116-
conflicts.extend(other.writes.ones());
142+
143+
if self.writes_all {
144+
conflicts.extend(other.reads_and_writes.ones());
117145
}
118146

119-
if other.reads_all {
120-
conflicts.extend(self.writes.ones());
147+
if other.writes_all {
148+
conflicts.extend(self.reads_and_writes.ones());
149+
}
150+
151+
if !(self.writes_all || other.writes_all) {
152+
match (self.reads_all, other.reads_all) {
153+
(false, false) => {
154+
conflicts.extend(self.writes.intersection(&other.reads_and_writes));
155+
conflicts.extend(self.reads_and_writes.intersection(&other.writes));
156+
}
157+
(false, true) => {
158+
conflicts.extend(self.writes.ones());
159+
}
160+
(true, false) => {
161+
conflicts.extend(other.writes.ones());
162+
}
163+
(true, true) => (),
164+
}
121165
}
122-
conflicts.extend(self.writes.intersection(&other.reads_and_writes));
123-
conflicts.extend(self.reads_and_writes.intersection(&other.writes));
166+
124167
conflicts
125168
.ones()
126169
.map(SparseSetIndex::get_sparse_set_index)
@@ -266,6 +309,11 @@ impl<T: SparseSetIndex> FilteredAccess<T> {
266309
pub fn read_all(&mut self) {
267310
self.access.read_all();
268311
}
312+
313+
/// Sets the underlying unfiltered access as having exclusive access to all indexed elements.
314+
pub fn write_all(&mut self) {
315+
self.access.write_all();
316+
}
269317
}
270318

271319
/// A collection of [`FilteredAccess`] instances.
@@ -306,6 +354,20 @@ impl<T: SparseSetIndex> FilteredAccessSet<T> {
306354
true
307355
}
308356

357+
/// Returns `true` if this and `filtered_access` can be active at the same time.
358+
pub fn is_compatible_single(&self, filtered_access: &FilteredAccess<T>) -> bool {
359+
if self.combined_access.is_compatible(filtered_access.access()) {
360+
return true;
361+
}
362+
for filtered in &self.filtered_accesses {
363+
if !filtered.is_compatible(filtered_access) {
364+
return false;
365+
}
366+
}
367+
368+
true
369+
}
370+
309371
/// Returns a vector of elements that this set and `other` cannot access at the same time.
310372
pub fn get_conflicts(&self, other: &FilteredAccessSet<T>) -> Vec<T> {
311373
// if the unfiltered access is incompatible, must check each pair
@@ -320,7 +382,7 @@ impl<T: SparseSetIndex> FilteredAccessSet<T> {
320382
conflicts.into_iter().collect()
321383
}
322384

323-
/// Returns a vector of elements that this set and `other` cannot access at the same time.
385+
/// Returns a vector of elements that this set and `filtered_access` cannot access at the same time.
324386
pub fn get_conflicts_single(&self, filtered_access: &FilteredAccess<T>) -> Vec<T> {
325387
// if the unfiltered access is incompatible, must check each pair
326388
let mut conflicts = HashSet::new();

crates/bevy_ecs/src/schedule/executor.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
use crate::{schedule::ParallelSystemContainer, world::World};
1+
use crate::{schedule::FunctionSystemContainer, world::World};
22
use downcast_rs::{impl_downcast, Downcast};
33

44
pub trait ParallelSystemExecutor: Downcast + Send + Sync {
55
/// Called by `SystemStage` whenever `systems` have been changed.
6-
fn rebuild_cached_data(&mut self, systems: &[ParallelSystemContainer]);
6+
fn rebuild_cached_data(&mut self, systems: &[FunctionSystemContainer]);
77

8-
fn run_systems(&mut self, systems: &mut [ParallelSystemContainer], world: &mut World);
8+
fn run_systems(&mut self, systems: &mut [FunctionSystemContainer], world: &mut World);
99
}
1010

1111
impl_downcast!(ParallelSystemExecutor);
@@ -14,9 +14,9 @@ impl_downcast!(ParallelSystemExecutor);
1414
pub struct SingleThreadedExecutor;
1515

1616
impl ParallelSystemExecutor for SingleThreadedExecutor {
17-
fn rebuild_cached_data(&mut self, _: &[ParallelSystemContainer]) {}
17+
fn rebuild_cached_data(&mut self, _: &[FunctionSystemContainer]) {}
1818

19-
fn run_systems(&mut self, systems: &mut [ParallelSystemContainer], world: &mut World) {
19+
fn run_systems(&mut self, systems: &mut [FunctionSystemContainer], world: &mut World) {
2020
for system in systems {
2121
if system.should_run() {
2222
#[cfg(feature = "trace")]

crates/bevy_ecs/src/schedule/executor_parallel.rs

+10-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use crate::{
22
archetype::ArchetypeComponentId,
33
query::Access,
4-
schedule::{ParallelSystemContainer, ParallelSystemExecutor},
4+
schedule::{FunctionSystemContainer, ParallelSystemExecutor},
5+
system::MaybeUnsafeCell,
56
world::World,
67
};
78
use async_channel::{Receiver, Sender};
@@ -74,14 +75,18 @@ impl Default for ParallelExecutor {
7475
}
7576

7677
impl ParallelSystemExecutor for ParallelExecutor {
77-
fn rebuild_cached_data(&mut self, systems: &[ParallelSystemContainer]) {
78+
fn rebuild_cached_data(&mut self, systems: &[FunctionSystemContainer]) {
7879
self.system_metadata.clear();
7980
self.queued.grow(systems.len());
8081
self.running.grow(systems.len());
8182
self.should_run.grow(systems.len());
8283

8384
// Construct scheduling data for systems.
8485
for container in systems {
86+
if container.system().is_exclusive() {
87+
panic!("executor cannot run exclusive systems");
88+
}
89+
8590
let dependencies_total = container.dependencies().len();
8691
let system = container.system();
8792
let (start_sender, start_receiver) = async_channel::bounded(1);
@@ -103,7 +108,7 @@ impl ParallelSystemExecutor for ParallelExecutor {
103108
}
104109
}
105110

106-
fn run_systems(&mut self, systems: &mut [ParallelSystemContainer], world: &mut World) {
111+
fn run_systems(&mut self, systems: &mut [FunctionSystemContainer], world: &mut World) {
107112
#[cfg(test)]
108113
if self.events_sender.is_none() {
109114
let (sender, receiver) = async_channel::unbounded::<SchedulingEvent>();
@@ -163,7 +168,7 @@ impl ParallelExecutor {
163168
fn prepare_systems<'scope>(
164169
&mut self,
165170
scope: &mut Scope<'scope, ()>,
166-
systems: &'scope mut [ParallelSystemContainer],
171+
systems: &'scope mut [FunctionSystemContainer],
167172
world: &'scope World,
168173
) {
169174
#[cfg(feature = "trace")]
@@ -191,7 +196,7 @@ impl ParallelExecutor {
191196
#[cfg(feature = "trace")]
192197
let system_guard = system_span.enter();
193198
// SAFETY: the executor prevents two systems with conflicting access from running simultaneously.
194-
unsafe { system.run_unsafe((), world) };
199+
unsafe { system.run_unsafe((), MaybeUnsafeCell::from_ref(world)) };
195200
#[cfg(feature = "trace")]
196201
drop(system_guard);
197202
finish_sender

0 commit comments

Comments
 (0)