Make System responsible for updating its own archetypes (#4115)

# Objective

- Make it possible to use `System`s outside of the scheduler/executor without having to define logic to track new archetypes and call `System::add_archetype()` for each.

## Solution

- Replace `System::add_archetype(&Archetype)` with `System::update_archetypes(&World)`, making systems responsible for tracking their own most recent archetype generation the way that `SystemState` already does.

This has minimal (or simplifying) effect on most of the code with the exception of `FunctionSystem`, which must now track the latest `ArchetypeGeneration` it saw instead of relying on the executor to do it.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
This commit is contained in:
Torne Wuff 2022-04-07 20:50:43 +00:00
parent 6f16580b8a
commit b1afe2dcca
15 changed files with 264 additions and 159 deletions

View File

@ -8,10 +8,15 @@ license = "MIT OR Apache-2.0"
[dev-dependencies]
glam = "0.20"
criterion = "0.3"
criterion = { version = "0.3", features = ["html_reports"] }
bevy_ecs = { path = "../crates/bevy_ecs" }
bevy_tasks = { path = "../crates/bevy_tasks" }
[[bench]]
name = "archetype_updates"
path = "benches/bevy_ecs/archetype_updates.rs"
harness = false
[[bench]]
name = "ecs_bench_suite"
path = "benches/bevy_ecs/ecs_bench_suite/mod.rs"

View File

@ -0,0 +1,119 @@
use bevy_ecs::{
component::Component,
schedule::{Stage, SystemStage},
world::World,
};
use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion};
criterion_group!(benches, no_archetypes, added_archetypes);
criterion_main!(benches);
#[derive(Component)]
struct A<const N: u16>(f32);
fn setup(system_count: usize) -> (World, SystemStage) {
let mut world = World::new();
fn empty() {}
let mut stage = SystemStage::parallel();
for _ in 0..system_count {
stage.add_system(empty);
}
stage.run(&mut world);
(world, stage)
}
/// create `count` entities with distinct archetypes
fn add_archetypes(world: &mut World, count: u16) {
for i in 0..count {
let mut e = world.spawn();
if i & 1 << 0 != 0 {
e.insert(A::<0>(1.0));
}
if i & 1 << 1 != 0 {
e.insert(A::<1>(1.0));
}
if i & 1 << 2 != 0 {
e.insert(A::<2>(1.0));
}
if i & 1 << 3 != 0 {
e.insert(A::<3>(1.0));
}
if i & 1 << 4 != 0 {
e.insert(A::<4>(1.0));
}
if i & 1 << 5 != 0 {
e.insert(A::<5>(1.0));
}
if i & 1 << 6 != 0 {
e.insert(A::<6>(1.0));
}
if i & 1 << 7 != 0 {
e.insert(A::<7>(1.0));
}
if i & 1 << 8 != 0 {
e.insert(A::<8>(1.0));
}
if i & 1 << 9 != 0 {
e.insert(A::<9>(1.0));
}
if i & 1 << 10 != 0 {
e.insert(A::<10>(1.0));
}
if i & 1 << 11 != 0 {
e.insert(A::<11>(1.0));
}
if i & 1 << 12 != 0 {
e.insert(A::<12>(1.0));
}
if i & 1 << 13 != 0 {
e.insert(A::<13>(1.0));
}
if i & 1 << 14 != 0 {
e.insert(A::<14>(1.0));
}
if i & 1 << 15 != 0 {
e.insert(A::<15>(1.0));
}
}
}
fn no_archetypes(criterion: &mut Criterion) {
let mut group = criterion.benchmark_group("no_archetypes");
for i in 0..=5 {
let system_count = i * 20;
let (mut world, mut stage) = setup(system_count);
group.bench_with_input(
BenchmarkId::new("system_count", system_count),
&system_count,
|bencher, &_system_count| {
bencher.iter(|| {
stage.run(&mut world);
});
},
);
}
}
fn added_archetypes(criterion: &mut Criterion) {
const SYSTEM_COUNT: usize = 100;
let mut group = criterion.benchmark_group("added_archetypes");
for archetype_count in [100, 200, 500, 1000, 2000, 5000, 10000] {
group.bench_with_input(
BenchmarkId::new("archetype_count", archetype_count),
&archetype_count,
|bencher, &archetype_count| {
bencher.iter_batched(
|| {
let (mut world, stage) = setup(SYSTEM_COUNT);
add_archetypes(&mut world, archetype_count);
(world, stage)
},
|(mut world, mut stage)| {
stage.run(&mut world);
},
criterion::BatchSize::LargeInput,
)
},
);
}
}

View File

@ -19,9 +19,7 @@ impl Benchmark {
let mut system = IntoSystem::into_system(query_system);
system.initialize(&mut world);
for archetype in world.archetypes().iter() {
system.new_archetype(archetype);
}
system.update_archetype_component_access(&world);
Self(world, entity, Box::new(system))
}

View File

@ -42,9 +42,7 @@ impl Benchmark {
world.insert_resource(TaskPool::default());
let mut system = IntoSystem::into_system(sys);
system.initialize(&mut world);
for archetype in world.archetypes().iter() {
system.new_archetype(archetype);
}
system.update_archetype_component_access(&world);
Self(world, Box::new(system))
}

View File

@ -37,9 +37,7 @@ impl Benchmark {
let mut system = IntoSystem::into_system(query_system);
system.initialize(&mut world);
for archetype in world.archetypes().iter() {
system.new_archetype(archetype);
}
system.update_archetype_component_access(&world);
Self(world, Box::new(system))
}

View File

@ -1,6 +1,6 @@
use crate::Time;
use bevy_ecs::{
archetype::{Archetype, ArchetypeComponentId},
archetype::ArchetypeComponentId,
component::ComponentId,
query::Access,
schedule::ShouldRun,
@ -177,10 +177,6 @@ impl System for FixedTimestep {
Cow::Borrowed(std::any::type_name::<FixedTimestep>())
}
fn new_archetype(&mut self, archetype: &Archetype) {
self.internal_system.new_archetype(archetype);
}
fn archetype_component_access(&self) -> &Access<ArchetypeComponentId> {
self.internal_system.archetype_component_access()
}
@ -220,6 +216,11 @@ impl System for FixedTimestep {
}
}
fn update_archetype_component_access(&mut self, world: &World) {
self.internal_system
.update_archetype_component_access(world);
}
fn check_change_tick(&mut self, change_tick: u32) {
self.internal_system.check_change_tick(change_tick);
}

View File

@ -1,4 +1,4 @@
use crate::{archetype::ArchetypeGeneration, schedule::ParallelSystemContainer, world::World};
use crate::{schedule::ParallelSystemContainer, world::World};
use downcast_rs::{impl_downcast, Downcast};
pub trait ParallelSystemExecutor: Downcast + Send + Sync {
@ -10,24 +10,13 @@ pub trait ParallelSystemExecutor: Downcast + Send + Sync {
impl_downcast!(ParallelSystemExecutor);
pub struct SingleThreadedExecutor {
/// Last archetypes generation observed by parallel systems.
archetype_generation: ArchetypeGeneration,
}
#[derive(Default)]
pub struct SingleThreadedExecutor;
impl Default for SingleThreadedExecutor {
fn default() -> Self {
Self {
archetype_generation: ArchetypeGeneration::initial(),
}
}
}
impl ParallelSystemExecutor for SingleThreadedExecutor {
fn rebuild_cached_data(&mut self, _: &[ParallelSystemContainer]) {}
fn run_systems(&mut self, systems: &mut [ParallelSystemContainer], world: &mut World) {
self.update_archetypes(systems, world);
for system in systems {
if system.should_run() {
#[cfg(feature = "trace")]
@ -38,21 +27,3 @@ impl ParallelSystemExecutor for SingleThreadedExecutor {
}
}
}
impl SingleThreadedExecutor {
/// Calls `system.new_archetype()` for each archetype added since the last call to
/// `update_archetypes` and updates cached `archetype_component_access`.
fn update_archetypes(&mut self, systems: &mut [ParallelSystemContainer], world: &World) {
let archetypes = world.archetypes();
let new_generation = archetypes.generation();
let old_generation = std::mem::replace(&mut self.archetype_generation, new_generation);
let archetype_index_range = old_generation.value()..new_generation.value();
for archetype in archetypes.archetypes[archetype_index_range].iter() {
for container in systems.iter_mut() {
let system = container.system_mut();
system.new_archetype(archetype);
}
}
}
}

View File

@ -1,5 +1,5 @@
use crate::{
archetype::{ArchetypeComponentId, ArchetypeGeneration},
archetype::ArchetypeComponentId,
query::Access,
schedule::{ParallelSystemContainer, ParallelSystemExecutor},
world::World,
@ -32,8 +32,6 @@ struct SystemSchedulingMetadata {
}
pub struct ParallelExecutor {
/// Last archetypes generation observed by parallel systems.
archetype_generation: ArchetypeGeneration,
/// Cached metadata of every system.
system_metadata: Vec<SystemSchedulingMetadata>,
/// Used by systems to notify the executor that they have finished.
@ -60,7 +58,6 @@ impl Default for ParallelExecutor {
fn default() -> Self {
let (finish_sender, finish_receiver) = async_channel::unbounded();
Self {
archetype_generation: ArchetypeGeneration::initial(),
system_metadata: Default::default(),
finish_sender,
finish_receiver,
@ -114,7 +111,17 @@ impl ParallelSystemExecutor for ParallelExecutor {
self.events_sender = Some(sender);
}
self.update_archetypes(systems, world);
{
#[cfg(feature = "trace")]
let _span = bevy_utils::tracing::info_span!("update_archetypes").entered();
for (index, container) in systems.iter_mut().enumerate() {
let meta = &mut self.system_metadata[index];
let system = container.system_mut();
system.update_archetype_component_access(world);
meta.archetype_component_access
.extend(system.archetype_component_access());
}
}
let compute_pool = world
.get_resource_or_insert_with(|| ComputeTaskPool(TaskPool::default()))
@ -154,27 +161,6 @@ impl ParallelSystemExecutor for ParallelExecutor {
}
impl ParallelExecutor {
/// Calls `system.new_archetype()` for each archetype added since the last call to
/// `update_archetypes` and updates cached `archetype_component_access`.
fn update_archetypes(&mut self, systems: &mut [ParallelSystemContainer], world: &World) {
#[cfg(feature = "trace")]
let _span = bevy_utils::tracing::info_span!("update_archetypes").entered();
let archetypes = world.archetypes();
let new_generation = archetypes.generation();
let old_generation = std::mem::replace(&mut self.archetype_generation, new_generation);
let archetype_index_range = old_generation.value()..new_generation.value();
for archetype in archetypes.archetypes[archetype_index_range].iter() {
for (index, container) in systems.iter_mut().enumerate() {
let meta = &mut self.system_metadata[index];
let system = container.system_mut();
system.new_archetype(archetype);
meta.archetype_component_access
.extend(system.archetype_component_access());
}
}
}
/// Populates `should_run` bitset, spawns tasks for systems that should run this iteration,
/// queues systems with no dependencies to run (or skip) at next opportunity.
fn prepare_systems<'scope>(

View File

@ -1,5 +1,5 @@
use crate::{
archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration},
archetype::ArchetypeComponentId,
component::ComponentId,
query::Access,
schedule::{BoxedRunCriteriaLabel, GraphNode, RunCriteriaLabel},
@ -44,20 +44,10 @@ pub enum ShouldRun {
NoAndCheckAgain,
}
#[derive(Default)]
pub(crate) struct BoxedRunCriteria {
criteria_system: Option<BoxedSystem<(), ShouldRun>>,
initialized: bool,
archetype_generation: ArchetypeGeneration,
}
impl Default for BoxedRunCriteria {
fn default() -> Self {
Self {
criteria_system: None,
initialized: false,
archetype_generation: ArchetypeGeneration::initial(),
}
}
}
impl BoxedRunCriteria {
@ -72,15 +62,6 @@ impl BoxedRunCriteria {
run_criteria.initialize(world);
self.initialized = true;
}
let archetypes = world.archetypes();
let new_generation = archetypes.generation();
let old_generation = std::mem::replace(&mut self.archetype_generation, new_generation);
let archetype_index_range = old_generation.value()..new_generation.value();
for archetype in archetypes.archetypes[archetype_index_range].iter() {
run_criteria.new_archetype(archetype);
}
let should_run = run_criteria.run((), world);
run_criteria.apply_buffers(world);
should_run
@ -104,7 +85,6 @@ pub(crate) struct RunCriteriaContainer {
pub(crate) label: Option<BoxedRunCriteriaLabel>,
pub(crate) before: Vec<BoxedRunCriteriaLabel>,
pub(crate) after: Vec<BoxedRunCriteriaLabel>,
archetype_generation: ArchetypeGeneration,
}
impl RunCriteriaContainer {
@ -118,7 +98,6 @@ impl RunCriteriaContainer {
label: descriptor.label,
before: descriptor.before,
after: descriptor.after,
archetype_generation: ArchetypeGeneration::initial(),
}
}
@ -135,25 +114,6 @@ impl RunCriteriaContainer {
RunCriteriaInner::Piped { system, .. } => system.initialize(world),
}
}
pub(crate) fn update_archetypes(&mut self, world: &World) {
let archetypes = world.archetypes();
let new_generation = archetypes.generation();
let old_generation = std::mem::replace(&mut self.archetype_generation, new_generation);
let archetype_index_range = old_generation.value()..new_generation.value();
for archetype in archetypes.archetypes[archetype_index_range].iter() {
match &mut self.inner {
RunCriteriaInner::Single(system) => {
system.new_archetype(archetype);
}
RunCriteriaInner::Piped { system, .. } => {
system.new_archetype(archetype);
}
}
}
self.archetype_generation = new_generation;
}
}
impl GraphNode for RunCriteriaContainer {
@ -380,8 +340,6 @@ impl System for RunOnce {
Cow::Borrowed(std::any::type_name::<RunOnce>())
}
fn new_archetype(&mut self, _archetype: &Archetype) {}
fn component_access(&self) -> &Access<ComponentId> {
&self.component_access
}
@ -407,5 +365,7 @@ impl System for RunOnce {
fn initialize(&mut self, _world: &mut World) {}
fn update_archetype_component_access(&mut self, _world: &World) {}
fn check_change_tick(&mut self, _change_tick: u32) {}
}

View File

@ -801,7 +801,6 @@ impl Stage for SystemStage {
for index in 0..self.run_criteria.len() {
let (run_criteria, tail) = self.run_criteria.split_at_mut(index);
let mut criteria = &mut tail[0];
criteria.update_archetypes(world);
match &mut criteria.inner {
RunCriteriaInner::Single(system) => criteria.should_run = system.run((), world),
RunCriteriaInner::Piped {
@ -901,7 +900,6 @@ impl Stage for SystemStage {
for index in 0..run_criteria.len() {
let (run_criteria, tail) = run_criteria.split_at_mut(index);
let criteria = &mut tail[0];
criteria.update_archetypes(world);
match criteria.should_run {
ShouldRun::No => (),
ShouldRun::Yes => criteria.should_run = ShouldRun::No,

View File

@ -1,5 +1,4 @@
use crate::{
archetype::ArchetypeGeneration,
system::{check_system_change_tick, BoxedSystem, IntoSystem},
world::World,
};
@ -70,7 +69,6 @@ where
pub struct ExclusiveSystemCoerced {
system: BoxedSystem<(), ()>,
archetype_generation: ArchetypeGeneration,
}
impl ExclusiveSystem for ExclusiveSystemCoerced {
@ -79,15 +77,6 @@ impl ExclusiveSystem for ExclusiveSystemCoerced {
}
fn run(&mut self, world: &mut World) {
let archetypes = world.archetypes();
let new_generation = archetypes.generation();
let old_generation = std::mem::replace(&mut self.archetype_generation, new_generation);
let archetype_index_range = old_generation.value()..new_generation.value();
for archetype in archetypes.archetypes[archetype_index_range].iter() {
self.system.new_archetype(archetype);
}
self.system.run((), world);
self.system.apply_buffers(world);
}
@ -108,7 +97,6 @@ where
fn exclusive_system(self) -> ExclusiveSystemCoerced {
ExclusiveSystemCoerced {
system: Box::new(IntoSystem::into_system(self)),
archetype_generation: ArchetypeGeneration::initial(),
}
}
}

View File

@ -1,5 +1,5 @@
use crate::{
archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId},
archetype::{ArchetypeComponentId, ArchetypeGeneration, ArchetypeId},
component::ComponentId,
prelude::FromWorld,
query::{Access, FilteredAccessSet},
@ -332,6 +332,8 @@ where
func: F,
param_state: Option<Param::Fetch>,
system_meta: SystemMeta,
world_id: Option<WorldId>,
archetype_generation: ArchetypeGeneration,
// NOTE: PhantomData<fn()-> T> gives this safe Send/Sync impls
#[allow(clippy::type_complexity)]
marker: PhantomData<fn() -> (In, Out, Marker)>,
@ -353,6 +355,8 @@ where
func,
param_state: None,
system_meta: SystemMeta::new::<F>(),
world_id: None,
archetype_generation: ArchetypeGeneration::initial(),
marker: PhantomData,
}
}
@ -374,12 +378,6 @@ where
self.system_meta.name.clone()
}
#[inline]
fn new_archetype(&mut self, archetype: &Archetype) {
let param_state = self.param_state.as_mut().unwrap();
param_state.new_archetype(archetype, &mut self.system_meta);
}
#[inline]
fn component_access(&self) -> &Access<ComponentId> {
self.system_meta.component_access_set.combined_access()
@ -417,12 +415,28 @@ where
#[inline]
fn initialize(&mut self, world: &mut World) {
self.world_id = Some(world.id());
self.param_state = Some(<Param::Fetch as SystemParamState>::init(
world,
&mut self.system_meta,
));
}
fn update_archetype_component_access(&mut self, world: &World) {
assert!(self.world_id == Some(world.id()), "Encountered a mismatched World. A System cannot be used with Worlds other than the one it was initialized with.");
let archetypes = world.archetypes();
let new_generation = archetypes.generation();
let old_generation = std::mem::replace(&mut self.archetype_generation, new_generation);
let archetype_index_range = old_generation.value()..new_generation.value();
for archetype_index in archetype_index_range {
self.param_state.as_mut().unwrap().new_archetype(
&archetypes[ArchetypeId::new(archetype_index)],
&mut self.system_meta,
);
}
}
#[inline]
fn check_change_tick(&mut self, change_tick: u32) {
check_system_change_tick(

View File

@ -96,7 +96,7 @@ mod tests {
use crate::{
self as bevy_ecs,
archetype::Archetypes,
archetype::{ArchetypeComponentId, Archetypes},
bundle::Bundles,
component::{Component, Components},
entity::{Entities, Entity},
@ -138,9 +138,6 @@ mod tests {
world.spawn().insert(A);
system.initialize(&mut world);
for archetype in world.archetypes.iter() {
system.new_archetype(archetype);
}
system.run((), &mut world);
}
@ -835,4 +832,78 @@ mod tests {
);
}
}
#[test]
fn update_archetype_component_access_works() {
use std::collections::HashSet;
fn a_not_b_system(_query: Query<&A, Without<B>>) {}
let mut world = World::default();
let mut system = IntoSystem::into_system(a_not_b_system);
let mut expected_ids = HashSet::<ArchetypeComponentId>::new();
let a_id = world.init_component::<A>();
// set up system and verify its access is empty
system.initialize(&mut world);
system.update_archetype_component_access(&world);
assert_eq!(
system
.archetype_component_access()
.reads()
.collect::<HashSet<_>>(),
expected_ids
);
// add some entities with archetypes that should match and save their ids
expected_ids.insert(
world
.spawn()
.insert_bundle((A,))
.archetype()
.get_archetype_component_id(a_id)
.unwrap(),
);
expected_ids.insert(
world
.spawn()
.insert_bundle((A, C))
.archetype()
.get_archetype_component_id(a_id)
.unwrap(),
);
// add some entities with archetypes that should not match
world.spawn().insert_bundle((A, B));
world.spawn().insert_bundle((B, C));
// update system and verify its accesses are correct
system.update_archetype_component_access(&world);
assert_eq!(
system
.archetype_component_access()
.reads()
.collect::<HashSet<_>>(),
expected_ids
);
// one more round
expected_ids.insert(
world
.spawn()
.insert_bundle((A, D))
.archetype()
.get_archetype_component_id(a_id)
.unwrap(),
);
world.spawn().insert_bundle((A, B, D));
system.update_archetype_component_access(&world);
assert_eq!(
system
.archetype_component_access()
.reads()
.collect::<HashSet<_>>(),
expected_ids
);
}
}

View File

@ -1,10 +1,7 @@
use bevy_utils::tracing::warn;
use crate::{
archetype::{Archetype, ArchetypeComponentId},
component::ComponentId,
query::Access,
schedule::SystemLabel,
archetype::ArchetypeComponentId, component::ComponentId, query::Access, schedule::SystemLabel,
world::World,
};
use std::borrow::Cow;
@ -28,8 +25,6 @@ pub trait System: Send + Sync + 'static {
type Out;
/// Returns the system's name.
fn name(&self) -> Cow<'static, str>;
/// Register a new archetype for this system.
fn new_archetype(&mut self, archetype: &Archetype);
/// Returns the system's component [`Access`].
fn component_access(&self) -> &Access<ComponentId>;
/// Returns the system's archetype component [`Access`].
@ -50,12 +45,15 @@ pub trait System: Send + Sync + 'static {
unsafe fn run_unsafe(&mut self, input: Self::In, world: &World) -> Self::Out;
/// Runs the system with the given input in the world.
fn run(&mut self, input: Self::In, world: &mut World) -> Self::Out {
self.update_archetype_component_access(world);
// SAFE: world and resources are exclusively borrowed
unsafe { self.run_unsafe(input, world) }
}
fn apply_buffers(&mut self, world: &mut World);
/// Initialize the system.
fn initialize(&mut self, _world: &mut World);
/// Update the system's archetype component [`Access`].
fn update_archetype_component_access(&mut self, world: &World);
fn check_change_tick(&mut self, change_tick: u32);
/// The default labels for the system
fn default_labels(&self) -> Vec<Box<dyn SystemLabel>> {

View File

@ -1,5 +1,5 @@
use crate::{
archetype::{Archetype, ArchetypeComponentId},
archetype::ArchetypeComponentId,
component::ComponentId,
query::Access,
system::{IntoSystem, System},
@ -60,16 +60,6 @@ impl<SystemA: System, SystemB: System<In = SystemA::Out>> System for ChainSystem
self.name.clone()
}
fn new_archetype(&mut self, archetype: &Archetype) {
self.system_a.new_archetype(archetype);
self.system_b.new_archetype(archetype);
self.archetype_component_access
.extend(self.system_a.archetype_component_access());
self.archetype_component_access
.extend(self.system_b.archetype_component_access());
}
fn archetype_component_access(&self) -> &Access<ArchetypeComponentId> {
&self.archetype_component_access
}
@ -101,6 +91,16 @@ impl<SystemA: System, SystemB: System<In = SystemA::Out>> System for ChainSystem
.extend(self.system_b.component_access());
}
fn update_archetype_component_access(&mut self, world: &World) {
self.system_a.update_archetype_component_access(world);
self.system_b.update_archetype_component_access(world);
self.archetype_component_access
.extend(self.system_a.archetype_component_access());
self.archetype_component_access
.extend(self.system_b.archetype_component_access());
}
fn check_change_tick(&mut self, change_tick: u32) {
self.system_a.check_change_tick(change_tick);
self.system_b.check_change_tick(change_tick);