[Merged by Bors] - Simplify design for *Labels

benches/benches/bevy_ecs/scheduling/schedule.rs

@@ -63,19 +63,29 @@ pub fn build_schedule(criterion: &mut Criterion) {
 
     // Use multiple different kinds of label to ensure that dynamic dispatch
     // doesn't somehow get optimized away.
-    #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, SystemLabel)]
+    #[derive(Debug, Clone, Copy)]
     struct NumLabel(usize);
-    #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, SystemLabel)]
+    #[derive(Debug, Clone, Copy, SystemLabel)]
     struct DummyLabel;
 
+    impl SystemLabel for NumLabel {
+        fn as_str(&self) -> &'static str {
+            let s = self.0.to_string();
+            Box::leak(s.into_boxed_str())
+        }
+    }
+
     let mut group = criterion.benchmark_group("build_schedule");
     group.warm_up_time(std::time::Duration::from_millis(500));
     group.measurement_time(std::time::Duration::from_secs(15));
 
     // Method: generate a set of `graph_size` systems which have a One True Ordering.
     // Add system to the stage with full constraints. Hopefully this should be maximimally
     // difficult for bevy to figure out.
-    let labels: Vec<_> = (0..1000).map(NumLabel).collect();
+    // Also, we are performing the `as_label` operation outside of the loop since that
+    // requires an allocation and a leak. This is not something that would be necessary in a
+    // real scenario, just a contrivance for the benchmark.
+    let labels: Vec<_> = (0..1000).map(|i| NumLabel(i).as_label()).collect();
 
     // Benchmark graphs of different sizes.
     for graph_size in [100, 500, 1000] {

crates/bevy_app/src/app.rs

@@ -56,7 +56,7 @@ pub struct App {
     pub runner: Box<dyn Fn(App)>,
     /// A container of [`Stage`]s set to be run in a linear order.
     pub schedule: Schedule,
-    sub_apps: HashMap<Box<dyn AppLabel>, SubApp>,
+    sub_apps: HashMap<AppLabelId, SubApp>,
 }
 
 /// Each `SubApp` has its own [`Schedule`] and [`World`], enabling a separation of concerns.
@@ -879,7 +879,7 @@ impl App {
         sub_app_runner: impl Fn(&mut World, &mut App) + 'static,
     ) -> &mut Self {
         self.sub_apps.insert(
-            Box::new(label),
+            label.as_label(),
             SubApp {
                 app,
                 runner: Box::new(sub_app_runner),
@@ -896,15 +896,16 @@ 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),
+            Err(label) => panic!("Sub-App with label '{:?}' does not exist", label.as_str()),
         }
     }
 
     /// Retrieves a `SubApp` inside this [`App`] with the given label, if it exists. Otherwise returns
     /// an [`Err`] containing the given label.
-    pub fn get_sub_app_mut(&mut self, label: impl AppLabel) -> Result<&mut App, impl AppLabel> {
+    pub fn get_sub_app_mut(&mut self, label: impl AppLabel) -> Result<&mut App, AppLabelId> {
+        let label = label.as_label();
         self.sub_apps
-            .get_mut((&label) as &dyn AppLabel)
+            .get_mut(&label)
             .map(|sub_app| &mut sub_app.app)
             .ok_or(label)
     }
@@ -917,15 +918,15 @@ 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),
+            Err(label) => panic!("Sub-App with label '{:?}' does not exist", label.as_str()),
         }
     }
 
     /// Retrieves a `SubApp` inside this [`App`] with the given label, if it exists. Otherwise returns
     /// an [`Err`] containing the given label.
     pub fn get_sub_app(&self, label: impl AppLabel) -> Result<&App, impl AppLabel> {
         self.sub_apps
-            .get((&label) as &dyn AppLabel)
+            .get(&label.as_label())
             .map(|sub_app| &sub_app.app)
             .ok_or(label)
     }

crates/bevy_ecs/src/lib.rs

@@ -34,9 +34,10 @@ pub mod prelude {
         event::{EventReader, EventWriter, Events},
         query::{Added, AnyOf, ChangeTrackers, Changed, Or, QueryState, With, Without},
         schedule::{
-            AmbiguitySetLabel, ExclusiveSystemDescriptorCoercion, ParallelSystemDescriptorCoercion,
-            RunCriteria, RunCriteriaDescriptorCoercion, RunCriteriaLabel, Schedule, Stage,
-            StageLabel, State, SystemLabel, SystemSet, SystemStage,
+            AmbiguitySetLabel, AmbiguitySetLabelId, ExclusiveSystemDescriptorCoercion,
+            ParallelSystemDescriptorCoercion, RunCriteria, RunCriteriaDescriptorCoercion,
+            RunCriteriaLabel, RunCriteriaLabelId, Schedule, Stage, StageLabel, StageLabelId, State,
+            SystemLabel, SystemLabelId, SystemSet, SystemStage,
         },
         system::{
             Commands, In, IntoChainSystem, IntoExclusiveSystem, IntoSystem, Local, NonSend,

crates/bevy_ecs/src/schedule/label.rs

@@ -5,8 +5,3 @@ define_label!(StageLabel);
 define_label!(SystemLabel);
 define_label!(AmbiguitySetLabel);
 define_label!(RunCriteriaLabel);
-
-pub(crate) type BoxedStageLabel = Box<dyn StageLabel>;
-pub(crate) type BoxedSystemLabel = Box<dyn SystemLabel>;
-pub(crate) type BoxedAmbiguitySetLabel = Box<dyn AmbiguitySetLabel>;
-pub(crate) type BoxedRunCriteriaLabel = Box<dyn RunCriteriaLabel>;

crates/bevy_ecs/src/schedule/mod.rs

@@ -38,8 +38,8 @@ use bevy_utils::HashMap;
 /// runs indefinitely.
 #[derive(Default)]
 pub struct Schedule {
-    stages: HashMap<BoxedStageLabel, Box<dyn Stage>>,
-    stage_order: Vec<BoxedStageLabel>,
+    stages: HashMap<StageLabelId, Box<dyn Stage>>,
+    stage_order: Vec<StageLabelId>,
     run_criteria: BoxedRunCriteria,
 }
 
@@ -109,9 +109,9 @@ impl Schedule {
     /// schedule.add_stage("my_stage", SystemStage::parallel());
     /// ```
     pub fn add_stage<S: Stage>(&mut self, label: impl StageLabel, stage: S) -> &mut Self {
-        let label: Box<dyn StageLabel> = Box::new(label);
-        self.stage_order.push(label.clone());
-        let prev = self.stages.insert(label.clone(), Box::new(stage));
+        let label = label.as_label();
+        self.stage_order.push(label);
+        let prev = self.stages.insert(label, Box::new(stage));
         assert!(prev.is_none(), "Stage already exists: {:?}.", label);
         self
     }
@@ -133,18 +133,18 @@ impl Schedule {
         label: impl StageLabel,
         stage: S,
     ) -> &mut Self {
-        let label: Box<dyn StageLabel> = Box::new(label);
-        let target = &target as &dyn StageLabel;
+        let label = label.as_label();
+        let target = target.as_label();
         let target_index = self
             .stage_order
             .iter()
             .enumerate()
-            .find(|(_i, stage_label)| &***stage_label == target)
+            .find(|(_i, stage_label)| **stage_label == target)
             .map(|(i, _)| i)
             .unwrap_or_else(|| panic!("Target stage does not exist: {:?}.", target));
 
-        self.stage_order.insert(target_index + 1, label.clone());
-        let prev = self.stages.insert(label.clone(), Box::new(stage));
+        self.stage_order.insert(target_index + 1, label);
+        let prev = self.stages.insert(label, Box::new(stage));
         assert!(prev.is_none(), "Stage already exists: {:?}.", label);
         self
     }
@@ -167,18 +167,18 @@ impl Schedule {
         label: impl StageLabel,
         stage: S,
     ) -> &mut Self {
-        let label: Box<dyn StageLabel> = Box::new(label);
-        let target = &target as &dyn StageLabel;
+        let label = label.as_label();
+        let target = target.as_label();
         let target_index = self
             .stage_order
             .iter()
             .enumerate()
-            .find(|(_i, stage_label)| &***stage_label == target)
+            .find(|(_i, stage_label)| **stage_label == target)
             .map(|(i, _)| i)
             .unwrap_or_else(|| panic!("Target stage does not exist: {:?}.", target));
 
-        self.stage_order.insert(target_index, label.clone());
-        let prev = self.stages.insert(label.clone(), Box::new(stage));
+        self.stage_order.insert(target_index, label);
+        let prev = self.stages.insert(label, Box::new(stage));
         assert!(prev.is_none(), "Stage already exists: {:?}.", label);
         self
     }
@@ -213,7 +213,7 @@ impl Schedule {
 
         let stage = self
             .get_stage_mut::<SystemStage>(&stage_label)
-            .unwrap_or_else(move || stage_not_found(&stage_label));
+            .unwrap_or_else(move || stage_not_found(&stage_label.as_label()));
         stage.add_system(system);
         self
     }
@@ -282,7 +282,10 @@ impl Schedule {
         func: F,
     ) -> &mut Self {
         let stage = self.get_stage_mut::<T>(&label).unwrap_or_else(move || {
-            panic!("stage '{:?}' does not exist or is the wrong type", label)
+            panic!(
+                "stage '{:?}' does not exist or is the wrong type",
+                label.as_label()
+            )
         });
         func(stage);
         self
@@ -305,7 +308,7 @@ impl Schedule {
     /// ```
     pub fn get_stage<T: Stage>(&self, label: &dyn StageLabel) -> Option<&T> {
         self.stages
-            .get(label)
+            .get(&label.as_label())
             .and_then(|stage| stage.downcast_ref::<T>())
     }
 
@@ -326,7 +329,7 @@ impl Schedule {
     /// ```
     pub fn get_stage_mut<T: Stage>(&mut self, label: &dyn StageLabel) -> Option<&mut T> {
         self.stages
-            .get_mut(label)
+            .get_mut(&label.as_label())
             .and_then(|stage| stage.downcast_mut::<T>())
     }
 
@@ -341,10 +344,10 @@ impl Schedule {
     }
 
     /// Iterates over all of schedule's stages and their labels, in execution order.
-    pub fn iter_stages(&self) -> impl Iterator<Item = (&dyn StageLabel, &dyn Stage)> {
+    pub fn iter_stages(&self) -> impl Iterator<Item = (StageLabelId, &dyn Stage)> {
         self.stage_order
             .iter()
-            .map(move |label| (&**label, &*self.stages[label]))
+            .map(move |&label| (label, &*self.stages[&label]))
     }
 }
 

crates/bevy_ecs/src/schedule/run_criteria.rs

@@ -1,5 +1,5 @@
 use crate::{
-    schedule::{BoxedRunCriteriaLabel, GraphNode, RunCriteriaLabel},
+    schedule::{GraphNode, RunCriteriaLabel, RunCriteriaLabelId},
     system::{BoxedSystem, IntoSystem, Local},
     world::World,
 };
@@ -94,9 +94,9 @@ pub(crate) enum RunCriteriaInner {
 pub(crate) struct RunCriteriaContainer {
     pub(crate) should_run: ShouldRun,
     pub(crate) inner: RunCriteriaInner,
-    pub(crate) label: Option<BoxedRunCriteriaLabel>,
-    pub(crate) before: Vec<BoxedRunCriteriaLabel>,
-    pub(crate) after: Vec<BoxedRunCriteriaLabel>,
+    pub(crate) label: Option<RunCriteriaLabelId>,
+    pub(crate) before: Vec<RunCriteriaLabelId>,
+    pub(crate) after: Vec<RunCriteriaLabelId>,
 }
 
 impl RunCriteriaContainer {
@@ -129,7 +129,7 @@ impl RunCriteriaContainer {
 }
 
 impl GraphNode for RunCriteriaContainer {
-    type Label = BoxedRunCriteriaLabel;
+    type Label = RunCriteriaLabelId;
 
     fn name(&self) -> Cow<'static, str> {
         match &self.inner {
@@ -138,26 +138,26 @@ impl GraphNode for RunCriteriaContainer {
         }
     }
 
-    fn labels(&self) -> &[BoxedRunCriteriaLabel] {
+    fn labels(&self) -> &[RunCriteriaLabelId] {
         if let Some(ref label) = self.label {
             std::slice::from_ref(label)
         } else {
             &[]
         }
     }
 
-    fn before(&self) -> &[BoxedRunCriteriaLabel] {
+    fn before(&self) -> &[RunCriteriaLabelId] {
         &self.before
     }
 
-    fn after(&self) -> &[BoxedRunCriteriaLabel] {
+    fn after(&self) -> &[RunCriteriaLabelId] {
         &self.after
     }
 }
 
 pub enum RunCriteriaDescriptorOrLabel {
     Descriptor(RunCriteriaDescriptor),
-    Label(BoxedRunCriteriaLabel),
+    Label(RunCriteriaLabelId),
 }
 
 #[derive(Clone, Copy)]
@@ -168,10 +168,10 @@ pub(crate) enum DuplicateLabelStrategy {
 
 pub struct RunCriteriaDescriptor {
     pub(crate) system: RunCriteriaSystem,
-    pub(crate) label: Option<BoxedRunCriteriaLabel>,
+    pub(crate) label: Option<RunCriteriaLabelId>,
     pub(crate) duplicate_label_strategy: DuplicateLabelStrategy,
-    pub(crate) before: Vec<BoxedRunCriteriaLabel>,
-    pub(crate) after: Vec<BoxedRunCriteriaLabel>,
+    pub(crate) before: Vec<RunCriteriaLabelId>,
+    pub(crate) after: Vec<RunCriteriaLabelId>,
 }
 
 pub(crate) enum RunCriteriaSystem {
@@ -212,12 +212,12 @@ where
     }
 }
 
-impl<L> IntoRunCriteria<BoxedRunCriteriaLabel> for L
+impl<L> IntoRunCriteria<RunCriteriaLabelId> for L
 where
     L: RunCriteriaLabel,
 {
     fn into(self) -> RunCriteriaDescriptorOrLabel {
-        RunCriteriaDescriptorOrLabel::Label(Box::new(self))
+        RunCriteriaDescriptorOrLabel::Label(self.as_label())
     }
 }
 
@@ -244,24 +244,24 @@ pub trait RunCriteriaDescriptorCoercion<Param> {
 
 impl RunCriteriaDescriptorCoercion<()> for RunCriteriaDescriptor {
     fn label(mut self, label: impl RunCriteriaLabel) -> RunCriteriaDescriptor {
-        self.label = Some(Box::new(label));
+        self.label = Some(label.as_label());
         self.duplicate_label_strategy = DuplicateLabelStrategy::Panic;
         self
     }
 
     fn label_discard_if_duplicate(mut self, label: impl RunCriteriaLabel) -> RunCriteriaDescriptor {
-        self.label = Some(Box::new(label));
+        self.label = Some(label.as_label());
         self.duplicate_label_strategy = DuplicateLabelStrategy::Discard;
         self
     }
 
     fn before(mut self, label: impl RunCriteriaLabel) -> RunCriteriaDescriptor {
-        self.before.push(Box::new(label));
+        self.before.push(label.as_label());
         self
     }
 
     fn after(mut self, label: impl RunCriteriaLabel) -> RunCriteriaDescriptor {
-        self.after.push(Box::new(label));
+        self.after.push(label.as_label());
         self
     }
 }
@@ -317,7 +317,7 @@ where
 }
 
 pub struct RunCriteria {
-    label: BoxedRunCriteriaLabel,
+    label: RunCriteriaLabelId,
 }
 
 impl RunCriteria {
@@ -332,7 +332,7 @@ impl RunCriteria {
             label: None,
             duplicate_label_strategy: DuplicateLabelStrategy::Panic,
             before: vec![],
-            after: vec![Box::new(label)],
+            after: vec![label.as_label()],
         }
     }
 }