From c397c2f81133d752196996c5bce865d19a3d4449 Mon Sep 17 00:00:00 2001 From: Cameron <51241057+maniwani@users.noreply.github.com> Date: Wed, 19 Jul 2023 13:44:34 -0700 Subject: [PATCH 1/2] interning --- crates/bevy_app/src/app.rs | 22 +- crates/bevy_app/src/main_schedule.rs | 2 +- crates/bevy_derive/src/lib.rs | 2 +- crates/bevy_ecs/macros/src/lib.rs | 6 +- crates/bevy_ecs/macros/src/set.rs | 6 +- crates/bevy_ecs/src/schedule/config.rs | 80 +++---- crates/bevy_ecs/src/schedule/graph_utils.rs | 8 +- crates/bevy_ecs/src/schedule/schedule.rs | 114 +++++----- crates/bevy_ecs/src/schedule/set.rs | 181 ++++++++++------ crates/bevy_ecs/src/system/combinator.rs | 3 +- .../src/system/exclusive_function_system.rs | 5 +- crates/bevy_ecs/src/system/function_system.rs | 5 +- crates/bevy_ecs/src/system/system.rs | 12 +- crates/bevy_ecs/src/world/error.rs | 4 +- crates/bevy_ecs/src/world/mod.rs | 16 +- crates/bevy_macro_utils/src/lib.rs | 136 +----------- crates/bevy_render/src/lib.rs | 2 +- crates/bevy_utils/src/label.rs | 195 ++++++++---------- 18 files changed, 351 insertions(+), 448 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 9b20116ad5913..3eb5e9ff3efd5 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -4,8 +4,7 @@ use bevy_ecs::{ prelude::*, schedule::{ apply_state_transition, common_conditions::run_once as run_once_condition, - run_enter_schedule, BoxedScheduleLabel, IntoSystemConfigs, IntoSystemSetConfigs, - ScheduleLabel, + run_enter_schedule, IntoSystemConfigs, IntoSystemSetConfigs, ScheduleId, ScheduleLabel, }, }; use bevy_utils::{tracing::debug, HashMap, HashSet}; @@ -70,7 +69,7 @@ pub struct App { /// The schedule that runs the main loop of schedule execution. /// /// This is initially set to [`Main`]. - pub main_schedule_label: BoxedScheduleLabel, + pub main_schedule_label: ScheduleId, sub_apps: HashMap, plugin_registry: Vec>, plugin_name_added: HashSet, @@ -156,7 +155,7 @@ impl SubApp { /// Runs the [`SubApp`]'s default schedule. pub fn run(&mut self) { - self.app.world.run_schedule(&*self.app.main_schedule_label); + self.app.world.run_schedule(self.app.main_schedule_label); self.app.world.clear_trackers(); } @@ -219,7 +218,7 @@ impl App { sub_apps: HashMap::default(), plugin_registry: Vec::default(), plugin_name_added: Default::default(), - main_schedule_label: Box::new(Main), + main_schedule_label: Main.as_label(), building_plugin_depth: 0, } } @@ -241,7 +240,7 @@ impl App { { #[cfg(feature = "trace")] let _bevy_main_update_span = info_span!("main app").entered(); - self.world.run_schedule(&*self.main_schedule_label); + self.world.run_schedule(self.main_schedule_label); } for (_label, sub_app) in self.sub_apps.iter_mut() { #[cfg(feature = "trace")] @@ -745,7 +744,7 @@ impl App { pub fn sub_app_mut(&mut self, label: impl AppLabel) -> &mut App { match self.get_sub_app_mut(label) { Ok(app) => app, - Err(label) => panic!("Sub-App with label '{:?}' does not exist", label.as_str()), + Err(label) => panic!("Sub-App with label '{:?}' does not exist", label), } } @@ -767,7 +766,7 @@ impl App { pub fn sub_app(&self, label: impl AppLabel) -> &App { match self.get_sub_app(label) { Ok(app) => app, - Err(label) => panic!("Sub-App with label '{:?}' does not exist", label.as_str()), + Err(label) => panic!("Sub-App with label '{:?}' does not exist", label), } } @@ -837,11 +836,12 @@ impl App { ) -> &mut Self { let mut schedules = self.world.resource_mut::(); - if schedules.get(&label).is_none() { - schedules.insert(label.dyn_clone(), Schedule::new()); + let id = ScheduleId::of(&label); + if !schedules.contains(&id) { + schedules.insert(id, Schedule::new()); } - let schedule = schedules.get_mut(&label).unwrap(); + let schedule = schedules.get_mut(&id).unwrap(); // Call the function f, passing in the schedule retrieved f(schedule); diff --git a/crates/bevy_app/src/main_schedule.rs b/crates/bevy_app/src/main_schedule.rs index 194185c366ae7..bcf18eeb30db3 100644 --- a/crates/bevy_app/src/main_schedule.rs +++ b/crates/bevy_app/src/main_schedule.rs @@ -125,7 +125,7 @@ impl MainScheduleOrder { let index = self .labels .iter() - .position(|current| (**current).eq(&after)) + .position(|current| (current.as_dyn_eq()).dyn_eq(after.as_dyn_eq())) .unwrap_or_else(|| panic!("Expected {after:?} to exist")); self.labels.insert(index + 1, Box::new(schedule)); } diff --git a/crates/bevy_derive/src/lib.rs b/crates/bevy_derive/src/lib.rs index 9030d8791ba71..02e443f863c08 100644 --- a/crates/bevy_derive/src/lib.rs +++ b/crates/bevy_derive/src/lib.rs @@ -208,5 +208,5 @@ pub fn derive_app_label(input: TokenStream) -> TokenStream { let input = syn::parse_macro_input!(input as syn::DeriveInput); let mut trait_path = BevyManifest::default().get_path("bevy_app"); trait_path.segments.push(format_ident!("AppLabel").into()); - derive_label(input, &trait_path, "app_label") + derive_label(input, &trait_path) } diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 110df68260981..1bda1db3fcf32 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -6,9 +6,7 @@ mod set; mod states; use crate::{fetch::derive_world_query_impl, set::derive_set}; -use bevy_macro_utils::{ - derive_boxed_label, ensure_no_collision, get_named_struct_fields, BevyManifest, -}; +use bevy_macro_utils::{derive_label, ensure_no_collision, get_named_struct_fields, BevyManifest}; use proc_macro::TokenStream; use proc_macro2::Span; use quote::{format_ident, quote}; @@ -435,7 +433,7 @@ pub fn derive_schedule_label(input: TokenStream) -> TokenStream { trait_path .segments .push(format_ident!("ScheduleLabel").into()); - derive_boxed_label(input, &trait_path) + derive_label(input, &trait_path) } /// Derive macro generating an impl of the trait `SystemSet`. diff --git a/crates/bevy_ecs/macros/src/set.rs b/crates/bevy_ecs/macros/src/set.rs index 66bfb601ecf35..fbcd4212a75cd 100644 --- a/crates/bevy_ecs/macros/src/set.rs +++ b/crates/bevy_ecs/macros/src/set.rs @@ -23,11 +23,7 @@ pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStrea ); (quote! { - impl #impl_generics #trait_path for #ident #ty_generics #where_clause { - fn dyn_clone(&self) -> std::boxed::Box { - std::boxed::Box::new(std::clone::Clone::clone(self)) - } - } + impl #impl_generics #trait_path for #ident #ty_generics #where_clause {} }) .into() } diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index 708661035ed55..850c66b38f30a 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -4,7 +4,7 @@ use crate::{ schedule::{ condition::{BoxedCondition, Condition}, graph_utils::{Ambiguity, Dependency, DependencyKind, GraphInfo}, - set::{BoxedSystemSet, IntoSystemSet, SystemSet}, + set::{IntoSystemSet, SystemSet, SystemSetUntyped}, }, system::{BoxedSystem, IntoSystem, System}, }; @@ -20,7 +20,7 @@ fn new_condition(condition: impl Condition) -> BoxedCondition { Box::new(condition_system) } -fn ambiguous_with(graph_info: &mut GraphInfo, set: BoxedSystemSet) { +fn ambiguous_with(graph_info: &mut GraphInfo, set: SystemSetUntyped) { match &mut graph_info.ambiguous_with { detection @ Ambiguity::Check => { *detection = Ambiguity::IgnoreWithSet(vec![set]); @@ -83,20 +83,20 @@ impl SystemConfigs { }) } - pub(crate) fn in_set_inner(&mut self, set: BoxedSystemSet) { + pub(crate) fn in_set_inner(&mut self, set: SystemSetUntyped) { match self { SystemConfigs::SystemConfig(config) => { config.graph_info.sets.push(set); } SystemConfigs::Configs { configs, .. } => { for config in configs { - config.in_set_inner(set.dyn_clone()); + config.in_set_inner(set); } } } } - fn before_inner(&mut self, set: BoxedSystemSet) { + fn before_inner(&mut self, set: SystemSetUntyped) { match self { SystemConfigs::SystemConfig(config) => { config @@ -106,13 +106,13 @@ impl SystemConfigs { } SystemConfigs::Configs { configs, .. } => { for config in configs { - config.before_inner(set.dyn_clone()); + config.before_inner(set); } } } } - fn after_inner(&mut self, set: BoxedSystemSet) { + fn after_inner(&mut self, set: SystemSetUntyped) { match self { SystemConfigs::SystemConfig(config) => { config @@ -122,7 +122,7 @@ impl SystemConfigs { } SystemConfigs::Configs { configs, .. } => { for config in configs { - config.after_inner(set.dyn_clone()); + config.after_inner(set); } } } @@ -141,14 +141,14 @@ impl SystemConfigs { } } - fn ambiguous_with_inner(&mut self, set: BoxedSystemSet) { + fn ambiguous_with_inner(&mut self, set: SystemSetUntyped) { match self { SystemConfigs::SystemConfig(config) => { ambiguous_with(&mut config.graph_info, set); } SystemConfigs::Configs { configs, .. } => { for config in configs { - config.ambiguous_with_inner(set.dyn_clone()); + config.ambiguous_with_inner(set); } } } @@ -302,25 +302,26 @@ impl IntoSystemConfigs<()> for SystemConfigs { #[track_caller] fn in_set(mut self, set: impl SystemSet) -> Self { + let set = SystemSetUntyped::of(&set); assert!( set.system_type().is_none(), "adding arbitrary systems to a system type set is not allowed" ); - self.in_set_inner(set.dyn_clone()); + self.in_set_inner(set); self } fn before(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); - self.before_inner(set.dyn_clone()); + let set = SystemSetUntyped::of(&set.into_system_set()); + self.before_inner(set); self } fn after(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); - self.after_inner(set.dyn_clone()); + let set = SystemSetUntyped::of(&set.into_system_set()); + self.after_inner(set); self } @@ -330,8 +331,8 @@ impl IntoSystemConfigs<()> for SystemConfigs { } fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); - self.ambiguous_with_inner(set.dyn_clone()); + let set = SystemSetUntyped::of(&set.into_system_set()); + self.ambiguous_with_inner(set); self } @@ -382,13 +383,13 @@ all_tuples!(impl_system_collection, 1, 20, P, S); /// A [`SystemSet`] with scheduling metadata. pub struct SystemSetConfig { - pub(super) set: BoxedSystemSet, + pub(super) set: SystemSetUntyped, pub(super) graph_info: GraphInfo, pub(super) conditions: Vec, } impl SystemSetConfig { - fn new(set: BoxedSystemSet) -> Self { + fn new(set: SystemSetUntyped) -> Self { // system type sets are automatically populated // to avoid unintentionally broad changes, they cannot be configured assert!( @@ -445,11 +446,11 @@ pub trait IntoSystemSetConfig: Sized { impl IntoSystemSetConfig for S { fn into_config(self) -> SystemSetConfig { - SystemSetConfig::new(Box::new(self)) + SystemSetConfig::new(SystemSetUntyped::of(&self)) } } -impl IntoSystemSetConfig for BoxedSystemSet { +impl IntoSystemSetConfig for SystemSetUntyped { fn into_config(self) -> SystemSetConfig { SystemSetConfig::new(self) } @@ -462,27 +463,28 @@ impl IntoSystemSetConfig for SystemSetConfig { #[track_caller] fn in_set(mut self, set: impl SystemSet) -> Self { + let set = SystemSetUntyped::of(&set); assert!( set.system_type().is_none(), "adding arbitrary systems to a system type set is not allowed" ); - self.graph_info.sets.push(Box::new(set)); + self.graph_info.sets.push(set); self } fn before(mut self, set: impl IntoSystemSet) -> Self { - self.graph_info.dependencies.push(Dependency::new( - DependencyKind::Before, - Box::new(set.into_system_set()), - )); + let set = SystemSetUntyped::of(&set.into_system_set()); + self.graph_info + .dependencies + .push(Dependency::new(DependencyKind::Before, set)); self } fn after(mut self, set: impl IntoSystemSet) -> Self { - self.graph_info.dependencies.push(Dependency::new( - DependencyKind::After, - Box::new(set.into_system_set()), - )); + let set = SystemSetUntyped::of(&set.into_system_set()); + self.graph_info + .dependencies + .push(Dependency::new(DependencyKind::After, set)); self } @@ -492,7 +494,8 @@ impl IntoSystemSetConfig for SystemSetConfig { } fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { - ambiguous_with(&mut self.graph_info, Box::new(set.into_system_set())); + let set = SystemSetUntyped::of(&set.into_system_set()); + ambiguous_with(&mut self.graph_info, set); self } @@ -561,45 +564,46 @@ impl IntoSystemSetConfigs for SystemSetConfigs { #[track_caller] fn in_set(mut self, set: impl SystemSet) -> Self { + let set = SystemSetUntyped::of(&set); assert!( set.system_type().is_none(), "adding arbitrary systems to a system type set is not allowed" ); for config in &mut self.sets { - config.graph_info.sets.push(set.dyn_clone()); + config.graph_info.sets.push(set); } self } fn before(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); + let set = SystemSetUntyped::of(&set.into_system_set()); for config in &mut self.sets { config .graph_info .dependencies - .push(Dependency::new(DependencyKind::Before, set.dyn_clone())); + .push(Dependency::new(DependencyKind::Before, set)); } self } fn after(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); + let set = SystemSetUntyped::of(&set.into_system_set()); for config in &mut self.sets { config .graph_info .dependencies - .push(Dependency::new(DependencyKind::After, set.dyn_clone())); + .push(Dependency::new(DependencyKind::After, set)); } self } fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); + let set = SystemSetUntyped::of(&set.into_system_set()); for config in &mut self.sets { - ambiguous_with(&mut config.graph_info, set.dyn_clone()); + ambiguous_with(&mut config.graph_info, set); } self diff --git a/crates/bevy_ecs/src/schedule/graph_utils.rs b/crates/bevy_ecs/src/schedule/graph_utils.rs index 9209ff05b94b8..852d7f00a8bb6 100644 --- a/crates/bevy_ecs/src/schedule/graph_utils.rs +++ b/crates/bevy_ecs/src/schedule/graph_utils.rs @@ -51,11 +51,11 @@ pub(crate) enum DependencyKind { #[derive(Clone)] pub(crate) struct Dependency { pub(crate) kind: DependencyKind, - pub(crate) set: BoxedSystemSet, + pub(crate) set: SystemSetUntyped, } impl Dependency { - pub fn new(kind: DependencyKind, set: BoxedSystemSet) -> Self { + pub fn new(kind: DependencyKind, set: SystemSetUntyped) -> Self { Self { kind, set } } } @@ -66,14 +66,14 @@ pub(crate) enum Ambiguity { #[default] Check, /// Ignore warnings with systems in any of these system sets. May contain duplicates. - IgnoreWithSet(Vec), + IgnoreWithSet(Vec), /// Ignore all warnings. IgnoreAll, } #[derive(Clone, Default)] pub(crate) struct GraphInfo { - pub(crate) sets: Vec, + pub(crate) sets: Vec, pub(crate) dependencies: Vec, pub(crate) ambiguous_with: Ambiguity, } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 701e13ff4578f..28c5b195f1ed5 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -23,10 +23,17 @@ use crate::{ world::World, }; +bevy_utils::define_label!( + /// Identifies a [`Schedule`](super::Schedule). + ScheduleLabel, + /// A lightweight and printable identifier for a [`Schedule`](super::Schedule). + ScheduleId, +); + /// Resource that stores [`Schedule`]s mapped to [`ScheduleLabel`]s. #[derive(Default, Resource)] pub struct Schedules { - inner: HashMap, + inner: HashMap, } impl Schedules { @@ -42,49 +49,47 @@ impl Schedules { /// If the map already had an entry for `label`, `schedule` is inserted, /// and the old schedule is returned. Otherwise, `None` is returned. pub fn insert(&mut self, label: impl ScheduleLabel, schedule: Schedule) -> Option { - let label = label.dyn_clone(); - self.inner.insert(label, schedule) + let id = label.as_label(); + if self.inner.contains_key(&id) { + warn!("schedule with label {:?} already exists", id); + } + self.inner.insert(id, schedule) } /// Removes the schedule corresponding to the `label` from the map, returning it if it existed. pub fn remove(&mut self, label: &dyn ScheduleLabel) -> Option { - self.inner.remove(label) - } - - /// Removes the (schedule, label) pair corresponding to the `label` from the map, returning it if it existed. - pub fn remove_entry( - &mut self, - label: &dyn ScheduleLabel, - ) -> Option<(Box, Schedule)> { - self.inner.remove_entry(label) + let id = label.as_label(); + if !self.inner.contains_key(&id) { + warn!("schedule with label {:?} not found", id); + } + self.inner.remove(&id) } - /// Does a schedule with the provided label already exist? + /// Does a schedule associated with the provided `label` already exist? pub fn contains(&self, label: &dyn ScheduleLabel) -> bool { - self.inner.contains_key(label) + let id = label.as_label(); + self.inner.contains_key(&id) } /// Returns a reference to the schedule associated with `label`, if it exists. pub fn get(&self, label: &dyn ScheduleLabel) -> Option<&Schedule> { - self.inner.get(label) + let id = label.as_label(); + self.inner.get(&id) } /// Returns a mutable reference to the schedule associated with `label`, if it exists. pub fn get_mut(&mut self, label: &dyn ScheduleLabel) -> Option<&mut Schedule> { - self.inner.get_mut(label) + let id = label.as_label(); + self.inner.get_mut(&id) } /// Returns an iterator over all schedules. Iteration order is undefined. - pub fn iter(&self) -> impl Iterator { - self.inner - .iter() - .map(|(label, schedule)| (&**label, schedule)) + pub fn iter(&self) -> impl Iterator { + self.inner.iter().map(|(id, schedule)| (*id, schedule)) } /// Returns an iterator over mutable references to all schedules. Iteration order is undefined. - pub fn iter_mut(&mut self) -> impl Iterator { - self.inner - .iter_mut() - .map(|(label, schedule)| (&**label, schedule)) + pub fn iter_mut(&mut self) -> impl Iterator { + self.inner.iter_mut().map(|(id, schedule)| (*id, schedule)) } /// Iterates the change ticks of all systems in all stored schedules and clamps any older than @@ -329,12 +334,12 @@ impl Dag { /// A [`SystemSet`] with metadata, stored in a [`ScheduleGraph`]. struct SystemSetNode { - inner: BoxedSystemSet, + inner: SystemSetUntyped, } impl SystemSetNode { - pub fn new(set: BoxedSystemSet) -> Self { - Self { inner: set } + pub fn new(info: SystemSetUntyped) -> Self { + Self { inner: info } } pub fn name(&self) -> String { @@ -378,7 +383,7 @@ pub struct ScheduleGraph { system_conditions: Vec>, system_sets: Vec, system_set_conditions: Vec>, - system_set_ids: HashMap, + system_set_ids: HashMap, uninit: Vec<(NodeId, usize)>, hierarchy: Dag, dependency: Dag, @@ -434,18 +439,18 @@ impl ScheduleGraph { } /// Returns the set at the given [`NodeId`], if it exists. - pub fn get_set_at(&self, id: NodeId) -> Option<&dyn SystemSet> { + pub fn get_set_at(&self, id: NodeId) -> Option<&SystemSetUntyped> { if !id.is_set() { return None; } - self.system_sets.get(id.index()).map(|set| &*set.inner) + self.system_sets.get(id.index()).map(|set| &set.inner) } /// Returns the set at the given [`NodeId`]. /// /// Panics if it doesn't exist. #[track_caller] - pub fn set_at(&self, id: NodeId) -> &dyn SystemSet { + pub fn set_at(&self, id: NodeId) -> &SystemSetUntyped { self.get_set_at(id) .ok_or_else(|| format!("set with id {id:?} does not exist in this Schedule")) .unwrap() @@ -466,10 +471,12 @@ impl ScheduleGraph { } /// Returns an iterator over all system sets in this schedule. - pub fn system_sets(&self) -> impl Iterator { + pub fn system_sets( + &self, + ) -> impl Iterator { self.system_set_ids.iter().map(|(_, &node_id)| { let set_node = &self.system_sets[node_id.index()]; - let set = &*set_node.inner; + let set = &set_node.inner; let conditions = self.system_set_conditions[node_id.index()].as_slice(); (node_id, set, conditions) }) @@ -532,7 +539,7 @@ impl ScheduleGraph { if more_than_one_entry { let set = AnonymousSet::new(); for config in &mut configs { - config.in_set_inner(set.dyn_clone()); + config.in_set_inner(SystemSetUntyped::of(&set)); } let mut set_config = set.into_config(); set_config.conditions.extend(collective_conditions); @@ -692,9 +699,9 @@ impl ScheduleGraph { mut conditions, } = set.into_config(); - let id = match self.system_set_ids.get(&set) { + let id = match self.system_set_ids.get(&set.id()) { Some(&id) => id, - None => self.add_set(set.dyn_clone()), + None => self.add_set(set), }; // graph updates are immediate @@ -708,23 +715,24 @@ impl ScheduleGraph { Ok(id) } - fn add_set(&mut self, set: BoxedSystemSet) -> NodeId { + fn add_set(&mut self, set: SystemSetUntyped) -> NodeId { let id = NodeId::Set(self.system_sets.len()); - self.system_sets.push(SystemSetNode::new(set.dyn_clone())); + self.system_sets.push(SystemSetNode::new(set)); self.system_set_conditions.push(Vec::new()); - self.system_set_ids.insert(set, id); + self.system_set_ids.insert(set.id(), id); id } - fn check_set(&mut self, id: &NodeId, set: &dyn SystemSet) -> Result<(), ScheduleBuildError> { - match self.system_set_ids.get(set) { - Some(set_id) => { - if id == set_id { - return Err(ScheduleBuildError::HierarchyLoop(self.get_node_name(id))); + fn check_set(&mut self, id: &NodeId, set: SystemSetUntyped) -> Result<(), ScheduleBuildError> { + match self.system_set_ids.get(&set.id()) { + Some(node_id) => { + if id == node_id { + let string = format!("{:?}", &set); + return Err(ScheduleBuildError::HierarchyLoop(string)); } } None => { - self.add_set(set.dyn_clone()); + self.add_set(set); } } @@ -737,7 +745,7 @@ impl ScheduleGraph { graph_info: &GraphInfo, ) -> Result<(), ScheduleBuildError> { for set in &graph_info.sets { - self.check_set(id, &**set)?; + self.check_set(id, *set)?; } Ok(()) @@ -749,22 +757,22 @@ impl ScheduleGraph { graph_info: &GraphInfo, ) -> Result<(), ScheduleBuildError> { for Dependency { kind: _, set } in &graph_info.dependencies { - match self.system_set_ids.get(set) { + match self.system_set_ids.get(&set.id()) { Some(set_id) => { if id == set_id { return Err(ScheduleBuildError::DependencyLoop(self.get_node_name(id))); } } None => { - self.add_set(set.dyn_clone()); + self.add_set(*set); } } } if let Ambiguity::IgnoreWithSet(ambiguous_with) = &graph_info.ambiguous_with { for set in ambiguous_with { - if !self.system_set_ids.contains_key(set) { - self.add_set(set.dyn_clone()); + if !self.system_set_ids.contains_key(&set.id()) { + self.add_set(*set); } } } @@ -791,7 +799,7 @@ impl ScheduleGraph { self.hierarchy.graph.add_node(id); self.dependency.graph.add_node(id); - for set in sets.into_iter().map(|set| self.system_set_ids[&set]) { + for set in sets.into_iter().map(|set| self.system_set_ids[&set.id()]) { self.hierarchy.graph.add_edge(set, id, ()); // ensure set also appears in dependency graph @@ -804,7 +812,7 @@ impl ScheduleGraph { for (kind, set) in dependencies .into_iter() - .map(|Dependency { kind, set }| (kind, self.system_set_ids[&set])) + .map(|Dependency { kind, set }| (kind, self.system_set_ids[&set.id()])) { let (lhs, rhs) = match kind { DependencyKind::Before => (id, set), @@ -821,7 +829,7 @@ impl ScheduleGraph { Ambiguity::IgnoreWithSet(ambiguous_with) => { for set in ambiguous_with .into_iter() - .map(|set| self.system_set_ids[&set]) + .map(|set| self.system_set_ids[&set.id()]) { self.ambiguous_with.add_edge(id, set, ()); } diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index c34745b9f34b6..e7e85fe77179c 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -5,61 +5,147 @@ use std::marker::PhantomData; use std::sync::atomic::{AtomicUsize, Ordering}; pub use bevy_ecs_macros::{ScheduleLabel, SystemSet}; -use bevy_utils::define_boxed_label; use bevy_utils::label::DynHash; use crate::system::{ ExclusiveSystemParamFunction, IsExclusiveFunctionSystem, IsFunctionSystem, SystemParamFunction, }; -define_boxed_label!(ScheduleLabel); - -/// A shorthand for `Box`. -pub type BoxedSystemSet = Box; -/// A shorthand for `Box`. -pub type BoxedScheduleLabel = Box; - -/// Types that identify logical groups of systems. +/// Identifies a logical group of systems. pub trait SystemSet: DynHash + Debug + Send + Sync + 'static { - /// Returns `Some` if this system set is a [`SystemTypeSet`]. + /// Returns the [`TypeId`] of the system if the system set is a [`SystemTypeSet`]. + /// + /// A [`SystemTypeSet`] has special properties: + /// - You cannot manually add systems or sets to it. + /// - You cannot configure it. + /// - You cannot order relative to it if it contains more than one instance. + /// + /// These sets are automatically populated, so these constraints exist to prevent unintentional ambiguity. fn system_type(&self) -> Option { None } - /// Returns `true` if this system set is an [`AnonymousSet`]. + /// Returns `true` if the system set is an [`AnonymousSet`]. fn is_anonymous(&self) -> bool { false } - /// Creates a boxed clone of the label corresponding to this system set. - fn dyn_clone(&self) -> Box; + + /// Returns the unique, type-elided identifier for the system set. + fn as_label(&self) -> SystemSetId { + SystemSetId::of(self) + } + + /// Returns the type-elided version of the system set. + fn as_untyped(&self) -> SystemSetUntyped { + SystemSetUntyped::of(self) + } } -impl PartialEq for dyn SystemSet { - fn eq(&self, other: &Self) -> bool { - self.dyn_eq(other.as_dyn_eq()) +/// A lightweight and printable identifier for a [`SystemSet`]. +#[derive(Clone, Copy, Eq)] +pub struct SystemSetId(&'static str); + +impl SystemSetId { + /// Returns the [`SystemSetId`] of the [`SystemSet`]. + pub fn of(set: &S) -> SystemSetId { + let str = bevy_utils::label::intern_debug_string(set); + SystemSetId(str) } } -impl Eq for dyn SystemSet {} +impl PartialEq for SystemSetId { + fn eq(&self, other: &Self) -> bool { + std::ptr::eq(self.0, other.0) + } +} -impl Hash for dyn SystemSet { +impl Hash for SystemSetId { fn hash(&self, state: &mut H) { - self.dyn_hash(state); + std::ptr::hash(self.0, state); } } -impl Clone for Box { - fn clone(&self) -> Self { - self.dyn_clone() +impl Debug for SystemSetId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + +/// The different variants of system sets. +#[derive(Clone, Copy, Eq, PartialEq, Hash)] +enum SystemSetKind { + Anonymous, + Named, + SystemType(TypeId), +} + +/// Type-elided struct whose methods return the same values as its original [`SystemSet`]. +#[derive(Clone, Copy, Eq, PartialEq, Hash)] + +pub struct SystemSetUntyped { + id: SystemSetId, + kind: SystemSetKind, +} + +impl SystemSetUntyped { + /// Converts a [`SystemSet`] into the equivalent [`SystemSetUntyped`]. + pub(crate) fn of(set: &S) -> SystemSetUntyped { + assert!(!(set.is_anonymous() && set.system_type().is_some())); + let kind = if let Some(type_id) = set.system_type() { + SystemSetKind::SystemType(type_id) + } else if set.is_anonymous() { + SystemSetKind::Anonymous + } else { + SystemSetKind::Named + }; + + SystemSetUntyped { + id: SystemSetId::of(set), + kind, + } + } + + /// Returns the [`TypeId`] of the system if the system set is a [`SystemTypeSet`]. + /// + /// A [`SystemTypeSet`] has special properties: + /// - You cannot manually add systems or sets to it. + /// - You cannot configure it. + /// - You cannot order relative to it if it contains more than one instance. + /// + /// These sets are automatically populated, so these constraints exist to prevent unintentional ambiguity. + pub fn system_type(&self) -> Option { + if let SystemSetKind::SystemType(type_id) = self.kind { + Some(type_id) + } else { + None + } + } + + /// Returns `true` if this system set is an [`AnonymousSet`]. + pub fn is_anonymous(&self) -> bool { + matches!(self.kind, SystemSetKind::Anonymous) + } + + /// Returns the [`SystemSetId`] of the set. + pub fn id(&self) -> SystemSetId { + self.id + } +} + +impl Debug for SystemSetUntyped { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.id.fmt(f) } } /// A [`SystemSet`] grouping instances of the same function. /// -/// This kind of set is automatically populated and thus has some special rules: -/// - You cannot manually add members. +/// These sets have special properties: +/// - You cannot manually add systems or sets to them. /// - You cannot configure them. -/// - You cannot order something relative to one if it has more than one member. +/// - You cannot order relative to one if it has more than one instance. +/// +/// These sets are automatically populated, so these constraints exist to prevent unintentional ambiguity. pub struct SystemTypeSet(PhantomData T>); impl SystemTypeSet { @@ -104,8 +190,8 @@ impl SystemSet for SystemTypeSet { Some(TypeId::of::()) } - fn dyn_clone(&self) -> Box { - Box::new(*self) + fn is_anonymous(&self) -> bool { + false } } @@ -126,10 +212,6 @@ impl SystemSet for AnonymousSet { fn is_anonymous(&self) -> bool { true } - - fn dyn_clone(&self) -> Box { - Box::new(*self) - } } /// Types that can be converted into a [`SystemSet`]. @@ -176,40 +258,3 @@ where SystemTypeSet::new() } } - -#[cfg(test)] -mod tests { - use crate::{ - schedule::{tests::ResMut, Schedule}, - system::Resource, - }; - - use super::*; - - #[test] - fn test_boxed_label() { - use crate::{self as bevy_ecs, world::World}; - - #[derive(Resource)] - struct Flag(bool); - - #[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] - struct A; - - let mut world = World::new(); - - let mut schedule = Schedule::new(); - schedule.add_systems(|mut flag: ResMut| flag.0 = true); - world.add_schedule(schedule, A); - - let boxed: Box = Box::new(A); - - world.insert_resource(Flag(false)); - world.run_schedule(&boxed); - assert!(world.resource::().0); - - world.insert_resource(Flag(false)); - world.run_schedule(boxed); - assert!(world.resource::().0); - } -} diff --git a/crates/bevy_ecs/src/system/combinator.rs b/crates/bevy_ecs/src/system/combinator.rs index 13c2b7869303d..1be776ea2bfe4 100644 --- a/crates/bevy_ecs/src/system/combinator.rs +++ b/crates/bevy_ecs/src/system/combinator.rs @@ -7,6 +7,7 @@ use crate::{ component::{ComponentId, Tick}, prelude::World, query::Access, + schedule::SystemSetUntyped, world::unsafe_world_cell::UnsafeWorldCell, }; @@ -226,7 +227,7 @@ where self.b.set_last_run(last_run); } - fn default_system_sets(&self) -> Vec> { + fn default_system_sets(&self) -> Vec { let mut default_sets = self.a.default_system_sets(); default_sets.append(&mut self.b.default_system_sets()); default_sets diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs index 6d2a31f554201..af3a65512410b 100644 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_function_system.rs @@ -2,6 +2,7 @@ use crate::{ archetype::ArchetypeComponentId, component::{ComponentId, Tick}, query::Access, + schedule::SystemSetUntyped, system::{ check_system_change_tick, ExclusiveSystemParam, ExclusiveSystemParamItem, In, IntoSystem, System, SystemMeta, @@ -147,9 +148,9 @@ where ); } - fn default_system_sets(&self) -> Vec> { + fn default_system_sets(&self) -> Vec { let set = crate::schedule::SystemTypeSet::::new(); - vec![Box::new(set)] + vec![SystemSetUntyped::of(&set)] } } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index e245f93716d78..e20c2160f21c7 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -3,6 +3,7 @@ use crate::{ component::{ComponentId, Tick}, prelude::FromWorld, query::{Access, FilteredAccessSet}, + schedule::SystemSetUntyped, system::{check_system_change_tick, ReadOnlySystemParam, System, SystemParam, SystemParamItem}, world::{unsafe_world_cell::UnsafeWorldCell, World, WorldId}, }; @@ -509,9 +510,9 @@ where ); } - fn default_system_sets(&self) -> Vec> { + fn default_system_sets(&self) -> Vec { let set = crate::schedule::SystemTypeSet::::new(); - vec![Box::new(set)] + vec![SystemSetUntyped::of(&set)] } } diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index 124aa4582fb00..d792bedb8f9b6 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -1,9 +1,13 @@ use bevy_utils::tracing::warn; use core::fmt::Debug; -use crate::component::Tick; -use crate::world::unsafe_world_cell::UnsafeWorldCell; -use crate::{archetype::ArchetypeComponentId, component::ComponentId, query::Access, world::World}; +use crate::{ + archetype::ArchetypeComponentId, + component::{ComponentId, Tick}, + query::Access, + schedule::SystemSetUntyped, + world::{unsafe_world_cell::UnsafeWorldCell, World}, +}; use std::any::TypeId; use std::borrow::Cow; @@ -89,7 +93,7 @@ pub trait System: Send + Sync + 'static { fn check_change_tick(&mut self, change_tick: Tick); /// Returns the system's default [system sets](crate::schedule::SystemSet). - fn default_system_sets(&self) -> Vec> { + fn default_system_sets(&self) -> Vec { Vec::new() } diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index f3794bb03d61c..5954630293832 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -2,11 +2,11 @@ use thiserror::Error; -use crate::schedule::BoxedScheduleLabel; +use crate::schedule::ScheduleId; /// The error type returned by [`World::try_run_schedule`] if the provided schedule does not exist. /// /// [`World::try_run_schedule`]: crate::world::World::try_run_schedule #[derive(Error, Debug)] #[error("The schedule with the label {0:?} was not found.")] -pub struct TryRunScheduleError(pub BoxedScheduleLabel); +pub struct TryRunScheduleError(pub ScheduleId); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index fae2931f7f898..80d4c1e8ca6a2 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1769,21 +1769,19 @@ impl World { label: impl AsRef, f: impl FnOnce(&mut World, &mut Schedule) -> R, ) -> Result { - let label = label.as_ref(); - let Some((extracted_label, mut schedule)) - = self.get_resource_mut::().and_then(|mut s| s.remove_entry(label)) + let label = label.as_ref().as_label(); + let Some(mut schedule) + = self.get_resource_mut::().and_then(|mut s| s.remove(&label)) else { - return Err(TryRunScheduleError(label.dyn_clone())); + return Err(TryRunScheduleError(label)); }; // TODO: move this span to Schedule::run #[cfg(feature = "trace")] - let _span = bevy_utils::tracing::info_span!("schedule", name = ?extracted_label).entered(); + let _span = bevy_utils::tracing::info_span!("schedule", name = ?label).entered(); let value = f(self, &mut schedule); - let old = self - .resource_mut::() - .insert(extracted_label, schedule); + let old = self.resource_mut::().insert(label, schedule); if old.is_some() { warn!("Schedule `{label:?}` was inserted during a call to `World::schedule_scope`: its value has been overwritten"); } @@ -1851,7 +1849,7 @@ impl World { self.try_schedule_scope(label, |world, sched| sched.run(world)) } - /// Runs the [`Schedule`] associated with the `label` a single time. + /// Runs the [`Schedule`] associated with the `id` a single time. /// /// The [`Schedule`] is fetched from the [`Schedules`] resource of the world by its label, /// and system state is cached. diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 59e224d51194d..33edef3300910 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -11,10 +11,10 @@ pub use shape::*; pub use symbol::*; use proc_macro::{TokenStream, TokenTree}; -use quote::{quote, quote_spanned}; +use quote::quote; use rustc_hash::FxHashSet; use std::{env, path::PathBuf}; -use syn::{spanned::Spanned, Ident}; +use syn::Ident; use toml_edit::{Document, Item}; pub struct BevyManifest { @@ -169,8 +169,8 @@ pub fn ensure_no_collision(value: Ident, haystack: TokenStream) -> Ident { /// /// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait /// - `trait_path`: The path [`syn::Path`] to the label trait -pub fn derive_boxed_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { - let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); +pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { + //let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); let ident = input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); @@ -186,21 +186,7 @@ pub fn derive_boxed_label(input: syn::DeriveInput, trait_path: &syn::Path) -> To ); (quote! { - impl #impl_generics #trait_path for #ident #ty_generics #where_clause { - fn dyn_clone(&self) -> std::boxed::Box { - std::boxed::Box::new(std::clone::Clone::clone(self)) - } - - fn as_dyn_eq(&self) -> &dyn #bevy_utils_path::label::DynEq { - self - } - - fn dyn_hash(&self, mut state: &mut dyn ::std::hash::Hasher) { - let ty_id = #trait_path::inner_type_id(self); - ::std::hash::Hash::hash(&ty_id, &mut state); - ::std::hash::Hash::hash(self, &mut state); - } - } + impl #impl_generics #trait_path for #ident #ty_generics #where_clause {} impl #impl_generics ::std::convert::AsRef for #ident #ty_generics #where_clause { fn as_ref(&self) -> &dyn #trait_path { @@ -210,115 +196,3 @@ pub fn derive_boxed_label(input: syn::DeriveInput, trait_path: &syn::Path) -> To }) .into() } - -/// Derive a label trait -/// -/// # Args -/// -/// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait -/// - `trait_path`: The path [`syn::Path`] to the label trait -pub fn derive_label( - input: syn::DeriveInput, - trait_path: &syn::Path, - attr_name: &str, -) -> TokenStream { - // return true if the variant specified is an `ignore_fields` attribute - fn is_ignore(attr: &syn::Attribute, attr_name: &str) -> bool { - if attr.path().get_ident().as_ref().unwrap() != &attr_name { - return false; - } - - syn::custom_keyword!(ignore_fields); - attr.parse_args_with(|input: syn::parse::ParseStream| { - let ignore = input.parse::>()?.is_some(); - Ok(ignore) - }) - .unwrap() - } - - let ident = input.ident.clone(); - - let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); - let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause { - where_token: Default::default(), - predicates: Default::default(), - }); - where_clause - .predicates - .push(syn::parse2(quote! { Self: 'static }).unwrap()); - - let as_str = match input.data { - syn::Data::Struct(d) => { - // see if the user tried to ignore fields incorrectly - if let Some(attr) = d - .fields - .iter() - .flat_map(|f| &f.attrs) - .find(|a| is_ignore(a, attr_name)) - { - let err_msg = format!("`#[{attr_name}(ignore_fields)]` cannot be applied to fields individually: add it to the struct declaration"); - return quote_spanned! { - attr.span() => compile_error!(#err_msg); - } - .into(); - } - // Structs must either be fieldless, or explicitly ignore the fields. - let ignore_fields = input.attrs.iter().any(|a| is_ignore(a, attr_name)); - if matches!(d.fields, syn::Fields::Unit) || ignore_fields { - let lit = ident.to_string(); - quote! { #lit } - } else { - let err_msg = format!("Labels cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`"); - return quote_spanned! { - d.fields.span() => compile_error!(#err_msg); - } - .into(); - } - } - syn::Data::Enum(d) => { - // check if the user put #[label(ignore_fields)] in the wrong place - if let Some(attr) = input.attrs.iter().find(|a| is_ignore(a, attr_name)) { - let err_msg = format!("`#[{attr_name}(ignore_fields)]` can only be applied to enum variants or struct declarations"); - return quote_spanned! { - attr.span() => compile_error!(#err_msg); - } - .into(); - } - let arms = d.variants.iter().map(|v| { - // Variants must either be fieldless, or explicitly ignore the fields. - let ignore_fields = v.attrs.iter().any(|a| is_ignore(a, attr_name)); - if matches!(v.fields, syn::Fields::Unit) | ignore_fields { - let mut path = syn::Path::from(ident.clone()); - path.segments.push(v.ident.clone().into()); - let lit = format!("{ident}::{}", v.ident.clone()); - quote! { #path { .. } => #lit } - } else { - let err_msg = format!("Label variants cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`"); - quote_spanned! { - v.fields.span() => _ => { compile_error!(#err_msg); } - } - } - }); - quote! { - match self { - #(#arms),* - } - } - } - syn::Data::Union(_) => { - return quote_spanned! { - input.span() => compile_error!("Unions cannot be used as labels."); - } - .into(); - } - }; - - (quote! { - impl #impl_generics #trait_path for #ident #ty_generics #where_clause { - fn as_str(&self) -> &'static str { - #as_str - } - } - }) - .into() -} diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index 61e814e9bc2d9..0ea1c0b31b843 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -265,7 +265,7 @@ impl Plugin for RenderPlugin { app.init_resource::(); let mut render_app = App::empty(); - render_app.main_schedule_label = Box::new(Render); + render_app.main_schedule_label = Render.as_label(); let mut extract_schedule = Schedule::new(); extract_schedule.set_apply_final_deferred(false); diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 6eff79c0bb389..31d5630496abc 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -1,9 +1,15 @@ //! Traits used by label implementations -use std::{ - any::Any, - hash::{Hash, Hasher}, -}; +use std::any::{Any, TypeId}; +use std::collections::hash_map::DefaultHasher; +use std::fmt::Debug; +use std::hash::{Hash, Hasher}; +use std::sync::{OnceLock, RwLock}; + +use crate::{default, HashMap}; + +type Interner = RwLock>; +static INTERNER: OnceLock = OnceLock::new(); /// An object safe version of [`Eq`]. This trait is automatically implemented /// for any `'static` type that implements `Eq`. @@ -60,90 +66,40 @@ where } } -/// Macro to define a new label trait -/// -/// # Example +/// The [`TypeId`](std::any::TypeId) of a value and its [`DynHash`] output +/// when hashed with the [`DefaultHasher`]. /// -/// ``` -/// # use bevy_utils::define_boxed_label; -/// define_boxed_label!(MyNewLabelTrait); -/// ``` -#[macro_export] -macro_rules! define_boxed_label { - ($label_trait_name:ident) => { - /// A strongly-typed label. - pub trait $label_trait_name: 'static + Send + Sync + ::std::fmt::Debug { - /// Return's the [TypeId] of this label, or the the ID of the - /// wrappped label type for `Box` - /// - /// [TypeId]: std::any::TypeId - fn inner_type_id(&self) -> ::std::any::TypeId { - std::any::TypeId::of::() - } - - /// Clones this ` - #[doc = stringify!($label_trait_name)] - /// ` - fn dyn_clone(&self) -> Box; - - /// Casts this value to a form where it can be compared with other type-erased values. - fn as_dyn_eq(&self) -> &dyn ::bevy_utils::label::DynEq; - - /// Feeds this value into the given [`Hasher`]. - /// - /// [`Hasher`]: std::hash::Hasher - fn dyn_hash(&self, state: &mut dyn ::std::hash::Hasher); - } +/// This is expected to have sufficient entropy to be different for each value. +#[derive(Clone, Copy, Eq, PartialEq, Hash)] +struct UniqueValue(TypeId, u64); - impl PartialEq for dyn $label_trait_name { - fn eq(&self, other: &Self) -> bool { - self.as_dyn_eq().dyn_eq(other.as_dyn_eq()) - } - } - - impl Eq for dyn $label_trait_name {} - - impl ::std::hash::Hash for dyn $label_trait_name { - fn hash(&self, state: &mut H) { - self.dyn_hash(state); - } - } - - impl ::std::convert::AsRef for dyn $label_trait_name { - #[inline] - fn as_ref(&self) -> &Self { - self - } - } - - impl Clone for Box { - fn clone(&self) -> Self { - self.dyn_clone() - } - } - - impl $label_trait_name for Box { - fn inner_type_id(&self) -> ::std::any::TypeId { - (**self).inner_type_id() - } +impl UniqueValue { + fn of(value: &T) -> Self { + let mut hasher = DefaultHasher::new(); + value.dyn_hash(&mut hasher); + let hash = hasher.finish(); - fn dyn_clone(&self) -> Box { - // Be explicit that we want to use the inner value - // to avoid infinite recursion. - (**self).dyn_clone() - } - - fn as_dyn_eq(&self) -> &dyn ::bevy_utils::label::DynEq { - (**self).as_dyn_eq() - } + Self(TypeId::of::(), hash) + } +} - fn dyn_hash(&self, state: &mut dyn ::std::hash::Hasher) { - (**self).dyn_hash(state); - } - } - }; +/// Returns a reference to the interned `Debug` string of `value`. +/// +/// If the string has not been interned, it will be allocated in a [`Box`] and leaked, but +/// subsequent calls with the same `value` will return the same reference. +pub fn intern_debug_string(value: &T) -> &'static str +where + T: Debug + DynHash + ?Sized, +{ + let key = UniqueValue::of(value); + let mut map = INTERNER.get_or_init(default).write().unwrap(); + let str = *map.entry(key).or_insert_with(|| { + let string = format!("{:?}", value); + let str: &'static str = Box::leak(string.into_boxed_str()); + str + }); + + str } /// Macro to define a new label trait @@ -168,49 +124,66 @@ macro_rules! define_label { $(#[$id_attr:meta])* $id_name:ident $(,)? ) => { + $(#[$label_attr])* + pub trait $label_name: bevy_utils::label::DynHash + ::std::fmt::Debug + Send + Sync + 'static { + /// Returns the unique, type-elided identifier for `self`. + fn as_label(&self) -> $id_name { + $id_name::of(self) + } + } + $(#[$id_attr])* - #[derive(Clone, Copy, PartialEq, Eq, Hash)] - pub struct $id_name(::core::any::TypeId, &'static str); + #[derive(Clone, Copy, Eq)] + pub struct $id_name(&'static str); + + impl $id_name { + /// Returns the [` + #[doc = stringify!($id_name)] + ///`] of the [` + #[doc = stringify!($label_name)] + ///`]. + pub fn of(label: &T) -> $id_name + where + T: $label_name + ?Sized, + { + $id_name(bevy_utils::label::intern_debug_string(label)) + } + } - impl ::core::fmt::Debug for $id_name { - fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { - write!(f, "{}", self.1) + impl ::std::cmp::PartialEq for $id_name { + fn eq(&self, other: &Self) -> bool { + ::std::ptr::eq(self.0, other.0) } } - $(#[$label_attr])* - pub trait $label_name: 'static { - /// Converts this type into an opaque, strongly-typed label. - fn as_label(&self) -> $id_name { - let id = self.type_id(); - let label = self.as_str(); - $id_name(id, label) + impl ::std::hash::Hash for $id_name { + fn hash(&self, state: &mut H) { + ::std::ptr::hash(self.0, state); } - /// Returns the [`TypeId`] used to differentiate labels. - fn type_id(&self) -> ::core::any::TypeId { - ::core::any::TypeId::of::() + } + + impl ::std::fmt::Debug for $id_name { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) } - /// Returns the representation of this label as a string literal. - /// - /// In cases where you absolutely need a label to be determined at runtime, - /// you can use [`Box::leak`] to get a `'static` reference. - fn as_str(&self) -> &'static str; } impl $label_name for $id_name { fn as_label(&self) -> Self { *self } - fn type_id(&self) -> ::core::any::TypeId { - self.0 - } - fn as_str(&self) -> &'static str { - self.1 + } + + impl ::std::convert::AsRef for $id_name { + #[inline] + fn as_ref(&self) -> &dyn $label_name { + self } } - impl $label_name for &'static str { - fn as_str(&self) -> Self { + impl ::std::convert::AsRef for dyn $label_name { + #[inline] + fn as_ref(&self) -> &dyn $label_name { self } } From 1c33f7dd8212dcd93bd92e0a5df5c8ec092df550 Mon Sep 17 00:00:00 2001 From: Cameron <51241057+maniwani@users.noreply.github.com> Date: Wed, 19 Jul 2023 16:09:45 -0700 Subject: [PATCH 2/2] Apply suggestions from code review --- crates/bevy_ecs/src/schedule/set.rs | 1 - crates/bevy_ecs/src/world/mod.rs | 2 +- crates/bevy_macro_utils/src/lib.rs | 2 -- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index e7e85fe77179c..92fa665716a07 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -81,7 +81,6 @@ enum SystemSetKind { /// Type-elided struct whose methods return the same values as its original [`SystemSet`]. #[derive(Clone, Copy, Eq, PartialEq, Hash)] - pub struct SystemSetUntyped { id: SystemSetId, kind: SystemSetKind, diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 80d4c1e8ca6a2..db48c91893824 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1849,7 +1849,7 @@ impl World { self.try_schedule_scope(label, |world, sched| sched.run(world)) } - /// Runs the [`Schedule`] associated with the `id` a single time. + /// Runs the [`Schedule`] associated with the `label` a single time. /// /// The [`Schedule`] is fetched from the [`Schedules`] resource of the world by its label, /// and system state is cached. diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 33edef3300910..5a4b41007e3df 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -170,8 +170,6 @@ pub fn ensure_no_collision(value: Ident, haystack: TokenStream) -> Ident { /// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait /// - `trait_path`: The path [`syn::Path`] to the label trait pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream { - //let bevy_utils_path = BevyManifest::default().get_path("bevy_utils"); - let ident = input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause {