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

Use string interning to avoid boxing labels #7760

Closed
wants to merge 2 commits into from
Closed
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
22 changes: 11 additions & 11 deletions crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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<AppLabelId, SubApp>,
plugin_registry: Vec<Box<dyn Plugin>>,
plugin_name_added: HashSet<String>,
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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,
}
}
Expand All @@ -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")]
Expand Down Expand Up @@ -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),
}
}

Expand All @@ -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),
}
}

Expand Down Expand Up @@ -837,11 +836,12 @@ impl App {
) -> &mut Self {
let mut schedules = self.world.resource_mut::<Schedules>();

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);

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_app/src/main_schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
6 changes: 2 additions & 4 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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`.
Expand Down
6 changes: 1 addition & 5 deletions crates/bevy_ecs/macros/src/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<dyn #trait_path> {
std::boxed::Box::new(std::clone::Clone::clone(self))
}
}
impl #impl_generics #trait_path for #ident #ty_generics #where_clause {}
})
.into()
}
80 changes: 42 additions & 38 deletions crates/bevy_ecs/src/schedule/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};
Expand All @@ -20,7 +20,7 @@ fn new_condition<M>(condition: impl Condition<M>) -> 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]);
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -122,7 +122,7 @@ impl SystemConfigs {
}
SystemConfigs::Configs { configs, .. } => {
for config in configs {
config.after_inner(set.dyn_clone());
config.after_inner(set);
}
}
}
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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<M>(mut self, set: impl IntoSystemSet<M>) -> 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<M>(mut self, set: impl IntoSystemSet<M>) -> 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
}

Expand All @@ -330,8 +331,8 @@ impl IntoSystemConfigs<()> for SystemConfigs {
}

fn ambiguous_with<M>(mut self, set: impl IntoSystemSet<M>) -> 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
}

Expand Down Expand Up @@ -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<BoxedCondition>,
}

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!(
Expand Down Expand Up @@ -445,11 +446,11 @@ pub trait IntoSystemSetConfig: Sized {

impl<S: SystemSet> 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)
}
Expand All @@ -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<M>(mut self, set: impl IntoSystemSet<M>) -> 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<M>(mut self, set: impl IntoSystemSet<M>) -> 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
}

Expand All @@ -492,7 +494,8 @@ impl IntoSystemSetConfig for SystemSetConfig {
}

fn ambiguous_with<M>(mut self, set: impl IntoSystemSet<M>) -> 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
}

Expand Down Expand Up @@ -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<M>(mut self, set: impl IntoSystemSet<M>) -> 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<M>(mut self, set: impl IntoSystemSet<M>) -> 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<M>(mut self, set: impl IntoSystemSet<M>) -> 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
Expand Down
Loading