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

Access control for non ecs data #16

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
9 changes: 5 additions & 4 deletions crates/bevy_core/src/time/fixed_timestep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::Time;
use bevy_ecs::{
archetype::{Archetype, ArchetypeComponentId},
component::ComponentId,
non_ecs_data::NonEcsDataId,
query::Access,
schedule::ShouldRun,
system::{ConfigurableSystem, IntoSystem, Local, Res, ResMut, System},
Expand Down Expand Up @@ -156,12 +157,12 @@ impl System for FixedTimestep {
self.internal_system.archetype_component_access()
}

fn component_access(&self) -> &Access<ComponentId> {
self.internal_system.component_access()
fn non_ecs_data_access(&self) -> &Access<NonEcsDataId> {
self.internal_system.non_ecs_data_access()
}

fn is_send(&self) -> bool {
self.internal_system.is_send()
fn component_access(&self) -> &Access<ComponentId> {
self.internal_system.component_access()
}

unsafe fn run_unsafe(&mut self, _input: (), world: &World) -> ShouldRun {
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pub mod change_detection;
pub mod component;
pub mod entity;
pub mod event;
pub mod non_ecs_data;
pub mod query;
#[cfg(feature = "bevy_reflect")]
pub mod reflect;
Expand Down
30 changes: 30 additions & 0 deletions crates/bevy_ecs/src/non_ecs_data.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use crate::storage::SparseSetIndex;

pub const NON_SEND_DATA_ID: usize = 0;
pub const ARCHETYPES_DATA_ID: usize = 1;

#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
pub struct NonEcsDataId(usize);

impl NonEcsDataId {
#[inline]
pub const fn new(index: usize) -> Self {
Self(index)
}

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

impl SparseSetIndex for NonEcsDataId {
#[inline]
fn sparse_set_index(&self) -> usize {
self.0
}

fn get_sparse_set_index(value: usize) -> Self {
Self(value)
}
}
43 changes: 25 additions & 18 deletions crates/bevy_ecs/src/schedule/executor_parallel.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{
archetype::{ArchetypeComponentId, ArchetypeGeneration},
non_ecs_data::{NonEcsDataId, NON_SEND_DATA_ID},
query::Access,
schedule::{ParallelSystemContainer, ParallelSystemExecutor},
world::World,
Expand All @@ -25,8 +26,8 @@ struct SystemSchedulingMetadata {
dependencies_now: usize,
/// Archetype-component access information.
archetype_component_access: Access<ArchetypeComponentId>,
/// Whether or not this system is send-able
is_send: bool,
/// Non-Ecs Data access information
non_ecs_data_access: Access<NonEcsDataId>,
}

pub struct ParallelExecutor {
Expand All @@ -42,12 +43,12 @@ pub struct ParallelExecutor {
queued: FixedBitSet,
/// Systems that are currently running.
running: FixedBitSet,
/// Whether a non-send system is currently running.
non_send_running: bool,
/// Systems that should run this iteration.
should_run: FixedBitSet,
/// Compound archetype-component access information of currently running systems.
active_archetype_component_access: Access<ArchetypeComponentId>,
/// Compound non ecs data access information of currently running systems.
active_non_ecs_data_access: Access<NonEcsDataId>,
/// Scratch space to avoid reallocating a vector when updating dependency counters.
dependants_scratch: Vec<usize>,
#[cfg(test)]
Expand All @@ -64,9 +65,9 @@ impl Default for ParallelExecutor {
finish_receiver,
queued: Default::default(),
running: Default::default(),
non_send_running: false,
should_run: Default::default(),
active_archetype_component_access: Default::default(),
active_non_ecs_data_access: Default::default(),
dependants_scratch: Default::default(),
#[cfg(test)]
events_sender: None,
Expand All @@ -92,8 +93,8 @@ impl ParallelSystemExecutor for ParallelExecutor {
dependants: vec![],
dependencies_total,
dependencies_now: 0,
is_send: system.is_send(),
archetype_component_access: Default::default(),
non_ecs_data_access: system.non_ecs_data_access().clone(),
});
}
// Populate the dependants lists in the scheduling metadata.
Expand Down Expand Up @@ -199,10 +200,13 @@ impl ParallelExecutor {
.await
.unwrap_or_else(|error| unreachable!(error));
};
if system_data.is_send {
scope.spawn(task);
} else {
if system_data
.non_ecs_data_access
.has_write(NonEcsDataId::new(NON_SEND_DATA_ID))
{
scope.spawn_local(task);
} else {
scope.spawn(task);
}
}
// Queue the system if it has no dependencies, otherwise reset its dependency counter.
Expand All @@ -218,10 +222,12 @@ impl ParallelExecutor {
fn can_start_now(&self, index: usize) -> bool {
let system_data = &self.system_metadata[index];
// Non-send systems are considered conflicting with each other.
(!self.non_send_running || system_data.is_send)
system_data
.archetype_component_access
.is_compatible(&self.active_archetype_component_access)
&& system_data
.archetype_component_access
.is_compatible(&self.active_archetype_component_access)
.non_ecs_data_access
.is_compatible(&self.active_non_ecs_data_access)
}

/// Starts all non-conflicting queued systems, moves them from `queued` to `running`,
Expand All @@ -247,12 +253,12 @@ impl ParallelExecutor {
.await
.unwrap_or_else(|error| unreachable!(error));
self.running.set(index, true);
if !system_metadata.is_send {
self.non_send_running = true;
}

// Add this system's access information to the active access information.
self.active_archetype_component_access
.extend(&system_metadata.archetype_component_access);
self.active_non_ecs_data_access
.extend(&system_metadata.non_ecs_data_access);
}
}
#[cfg(test)]
Expand All @@ -269,9 +275,6 @@ impl ParallelExecutor {
/// in the `dependants_scratch`.
fn process_finished_system(&mut self, index: usize) {
let system_data = &self.system_metadata[index];
if !system_data.is_send {
self.non_send_running = false;
}
self.running.set(index, false);
self.dependants_scratch.extend(&system_data.dependants);
}
Expand All @@ -280,9 +283,13 @@ impl ParallelExecutor {
/// running systems' access information.
fn rebuild_active_access(&mut self) {
self.active_archetype_component_access.clear();
self.active_non_ecs_data_access.clear();
for index in self.running.ones() {
self.active_archetype_component_access
.extend(&self.system_metadata[index].archetype_component_access);

self.active_non_ecs_data_access
.extend(&self.system_metadata[index].non_ecs_data_access);
}
}

Expand Down
17 changes: 15 additions & 2 deletions crates/bevy_ecs/src/schedule/run_criteria.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::{
archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration},
component::ComponentId,
non_ecs_data::NonEcsDataId,
query::Access,
schedule::{BoxedRunCriteriaLabel, GraphNode, RunCriteriaLabel},
system::{BoxedSystem, IntoSystem, System},
Expand Down Expand Up @@ -400,9 +401,21 @@ where
pub struct RunOnce {
ran: bool,
archetype_component_access: Access<ArchetypeComponentId>,
non_ecs_data_access: Access<NonEcsDataId>,
component_access: Access<ComponentId>,
}

impl Default for RunOnce {
fn default() -> Self {
Self {
ran: false,
archetype_component_access: Default::default(),
non_ecs_data_access: Default::default(),
component_access: Default::default(),
}
}
}

impl System for RunOnce {
type In = ();
type Out = ShouldRun;
Expand All @@ -421,8 +434,8 @@ impl System for RunOnce {
&self.archetype_component_access
}

fn is_send(&self) -> bool {
true
fn non_ecs_data_access(&self) -> &Access<NonEcsDataId> {
&self.non_ecs_data_access
}

unsafe fn run_unsafe(&mut self, _input: (), _world: &World) -> ShouldRun {
Expand Down
26 changes: 5 additions & 21 deletions crates/bevy_ecs/src/system/function_system.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::{
archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId},
component::ComponentId,
non_ecs_data::NonEcsDataId,
query::{Access, FilteredAccessSet},
system::{
check_system_change_tick, ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch,
Expand All @@ -16,9 +17,7 @@ pub struct SystemMeta {
pub(crate) name: Cow<'static, str>,
pub(crate) component_access_set: FilteredAccessSet<ComponentId>,
pub(crate) archetype_component_access: Access<ArchetypeComponentId>,
// NOTE: this must be kept private. making a SystemMeta non-send is irreversible to prevent
// SystemParams from overriding each other
is_send: bool,
pub(crate) non_ecs_data_access: Access<NonEcsDataId>,
pub(crate) last_change_tick: u32,
}

Expand All @@ -27,26 +26,11 @@ impl SystemMeta {
Self {
name: std::any::type_name::<T>().into(),
archetype_component_access: Access::default(),
non_ecs_data_access: Access::default(),
component_access_set: FilteredAccessSet::default(),
is_send: true,
last_change_tick: 0,
}
}

/// Returns true if the system is [`Send`].
#[inline]
pub fn is_send(&self) -> bool {
self.is_send
}

/// Sets the system to be not [`Send`].
///
/// This is irreversible.
#[inline]
pub fn set_non_send(&mut self) {
self.is_send = false;
}

#[inline]
pub(crate) fn check_change_tick(&mut self, change_tick: u32) {
check_system_change_tick(&mut self.last_change_tick, change_tick, self.name.as_ref());
Expand Down Expand Up @@ -432,8 +416,8 @@ where
}

#[inline]
fn is_send(&self) -> bool {
self.system_meta.is_send
fn non_ecs_data_access(&self) -> &Access<NonEcsDataId> {
&self.system_meta.non_ecs_data_access
}

#[inline]
Expand Down
5 changes: 3 additions & 2 deletions crates/bevy_ecs/src/system/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use bevy_utils::tracing::warn;
use crate::{
archetype::{Archetype, ArchetypeComponentId},
component::ComponentId,
non_ecs_data::NonEcsDataId,
query::Access,
world::World,
};
Expand Down Expand Up @@ -32,8 +33,8 @@ pub trait System: Send + Sync + 'static {
fn component_access(&self) -> &Access<ComponentId>;
/// Returns the system's archetype component [`Access`].
fn archetype_component_access(&self) -> &Access<ArchetypeComponentId>;
/// Returns true if the system is [`Send`].
fn is_send(&self) -> bool;
/// Returns the non ecs data ['Access]
fn non_ecs_data_access(&self) -> &Access<NonEcsDataId>;
/// Runs the system with the given input in the world. Unlike [`System::run`], this function
/// takes a shared reference to [`World`] and may therefore break Rust's aliasing rules, making
/// it unsafe to call.
Expand Down
16 changes: 12 additions & 4 deletions crates/bevy_ecs/src/system/system_chaining.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::{
archetype::{Archetype, ArchetypeComponentId},
component::ComponentId,
non_ecs_data::NonEcsDataId,
query::Access,
system::{IntoSystem, System},
world::World,
Expand Down Expand Up @@ -50,6 +51,7 @@ pub struct ChainSystem<SystemA, SystemB> {
name: Cow<'static, str>,
component_access: Access<ComponentId>,
archetype_component_access: Access<ArchetypeComponentId>,
non_ecs_data_access: Access<NonEcsDataId>,
}

impl<SystemA: System, SystemB: System<In = SystemA::Out>> System for ChainSystem<SystemA, SystemB> {
Expand All @@ -74,12 +76,12 @@ impl<SystemA: System, SystemB: System<In = SystemA::Out>> System for ChainSystem
&self.archetype_component_access
}

fn component_access(&self) -> &Access<ComponentId> {
&self.component_access
fn non_ecs_data_access(&self) -> &Access<NonEcsDataId> {
&self.non_ecs_data_access
}

fn is_send(&self) -> bool {
self.system_a.is_send() && self.system_b.is_send()
fn component_access(&self) -> &Access<ComponentId> {
&self.component_access
}

unsafe fn run_unsafe(&mut self, input: Self::In, world: &World) -> Self::Out {
Expand All @@ -99,6 +101,11 @@ impl<SystemA: System, SystemB: System<In = SystemA::Out>> System for ChainSystem
.extend(self.system_a.component_access());
self.component_access
.extend(self.system_b.component_access());

self.non_ecs_data_access
.extend(self.system_a.non_ecs_data_access());
self.non_ecs_data_access
.extend(self.system_b.non_ecs_data_access());
}

fn check_change_tick(&mut self, change_tick: u32) {
Expand Down Expand Up @@ -137,6 +144,7 @@ where
system_a,
system_b,
archetype_component_access: Default::default(),
non_ecs_data_access: Default::default(),
component_access: Default::default(),
}
}
Expand Down
9 changes: 7 additions & 2 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
change_detection::Ticks,
component::{Component, ComponentId, ComponentTicks, Components},
entity::{Entities, Entity},
non_ecs_data::{NonEcsDataId, NON_SEND_DATA_ID},
query::{
FilterFetch, FilteredAccess, FilteredAccessSet, QueryState, ReadOnlyFetch, WorldQuery,
},
Expand Down Expand Up @@ -771,7 +772,9 @@ unsafe impl<T: 'static> SystemParamState for NonSendState<T> {
type Config = ();

fn init(world: &mut World, system_meta: &mut SystemMeta, _config: Self::Config) -> Self {
system_meta.set_non_send();
system_meta
.non_ecs_data_access
.add_write(NonEcsDataId::new(NON_SEND_DATA_ID));

let component_id = world.initialize_non_send_resource::<T>();
let combined_access = system_meta.component_access_set.combined_access_mut();
Expand Down Expand Up @@ -887,7 +890,9 @@ unsafe impl<T: 'static> SystemParamState for NonSendMutState<T> {
type Config = ();

fn init(world: &mut World, system_meta: &mut SystemMeta, _config: Self::Config) -> Self {
system_meta.set_non_send();
system_meta
.non_ecs_data_access
.add_write(NonEcsDataId::new(NON_SEND_DATA_ID));

let component_id = world.initialize_non_send_resource::<T>();
let combined_access = system_meta.component_access_set.combined_access_mut();
Expand Down