From d7cf9186335ea3a7e70bcc1d707f5cd44defb129 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 13 Aug 2022 14:15:10 -0400 Subject: [PATCH 01/19] Add types for statically interning values --- crates/bevy_utils/Cargo.toml | 2 ++ crates/bevy_utils/src/intern.rs | 52 +++++++++++++++++++++++++++++++++ crates/bevy_utils/src/lib.rs | 2 ++ 3 files changed, 56 insertions(+) create mode 100644 crates/bevy_utils/src/intern.rs diff --git a/crates/bevy_utils/Cargo.toml b/crates/bevy_utils/Cargo.toml index 49c67fc021f5c..1860b7ad0408f 100644 --- a/crates/bevy_utils/Cargo.toml +++ b/crates/bevy_utils/Cargo.toml @@ -14,6 +14,8 @@ tracing = { version = "0.1", default-features = false, features = ["std"] } instant = { version = "0.1", features = ["wasm-bindgen"] } uuid = { version = "1.1", features = ["v4", "serde"] } hashbrown = { version = "0.12", features = ["serde"] } +indexmap = "1.9" +parking_lot = "0.12" [target.'cfg(target_arch = "wasm32")'.dependencies] getrandom = {version = "0.2.0", features = ["js"]} diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs new file mode 100644 index 0000000000000..edf9b65c0bc3f --- /dev/null +++ b/crates/bevy_utils/src/intern.rs @@ -0,0 +1,52 @@ +use std::hash::Hash; + +use parking_lot::{RwLock, RwLockReadGuard}; + +use crate::FixedState; + +type IndexSet = indexmap::IndexSet; + +/// A data structure used to intern a set of values of a specific type. +/// To store multiple distinct types, or generic types, try [`AnyInterner`]. +pub struct Interner( + // The `IndexSet` is a hash set that preserves ordering as long as + // you don't remove items (which we don't). + // This allows us to have O(~1) hashing and map each entry to a stable index. + RwLock>, +); + +/// The type returned from [`Interner::get`](Interner#method.get). +/// +/// Will hold a lock on the interner until this guard gets dropped. +pub type InternGuard<'a, L> = parking_lot::MappedRwLockReadGuard<'a, L>; + +impl Interner { + pub const fn new() -> Self { + Self(RwLock::new(IndexSet::with_hasher(FixedState))) + } + + /// Interns a value, if it was not already interned in this set. + /// + /// Returns an integer used to refer to the value later on. + pub fn intern(&self, val: &T) -> usize { + use parking_lot::RwLockUpgradableReadGuard as Guard; + + // Acquire an upgradeable read lock, since we might not have to do any writing. + let set = self.0.upgradable_read(); + + // If the value is already interned, return its index. + if let Some(idx) = set.get_index_of(val) { + return idx; + } + + // Upgrade to a mutable lock. + let mut set = Guard::upgrade(set); + let (idx, _) = set.insert_full(val.clone()); + idx + } + + /// Gets a reference to the value with specified index. + pub fn get(&self, idx: usize) -> Option> { + RwLockReadGuard::try_map(self.0.read(), |set| set.get_index(idx)).ok() + } +} diff --git a/crates/bevy_utils/src/lib.rs b/crates/bevy_utils/src/lib.rs index 2b09fdbf0f3b2..1c5b2d2e6b021 100644 --- a/crates/bevy_utils/src/lib.rs +++ b/crates/bevy_utils/src/lib.rs @@ -9,12 +9,14 @@ pub use short_names::get_short_name; mod default; mod float_ord; +mod intern; pub use ahash::AHasher; pub use default::default; pub use float_ord::*; pub use hashbrown; pub use instant::{Duration, Instant}; +pub use intern::*; pub use tracing; pub use uuid::Uuid; From 9ccea8e42d51e38b31650eaf532e7a380140b7d2 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 13 Aug 2022 14:43:16 -0400 Subject: [PATCH 02/19] allow arbitrary formatting using a fn pointer --- crates/bevy_utils/src/label.rs | 103 ++++++++++++++++++++++++++------- 1 file changed, 82 insertions(+), 21 deletions(-) diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 835656569c1fe..bb25bb8f408d1 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -5,6 +5,8 @@ use std::{ hash::{Hash, Hasher}, }; +use crate::Interner; + pub trait DynEq: Any { fn as_any(&self) -> &dyn Any; @@ -47,6 +49,30 @@ where } } +#[doc(hidden)] +pub struct VTable { + // FIXME: When const TypeId stabilizes, inline the type instead of using a fn pointer for indirection. + pub ty: fn() -> ::std::any::TypeId, + pub fmt: fn(u64, &mut ::std::fmt::Formatter) -> ::std::fmt::Result, +} + +impl PartialEq for VTable { + #[inline] + fn eq(&self, other: &Self) -> bool { + (self.ty)() == (other.ty)() + } +} +impl Eq for VTable {} + +impl Hash for VTable { + fn hash(&self, state: &mut H) { + (self.ty)().hash(state); + } +} + +#[doc(hidden)] +pub static STR_INTERN: Interner<&str> = Interner::new(); + /// Macro to define a new label trait /// /// # Example @@ -71,48 +97,83 @@ macro_rules! define_label { ) => { $(#[$id_attr])* #[derive(Clone, Copy, PartialEq, Eq, Hash)] - pub struct $id_name(::core::any::TypeId, &'static str); + pub struct $id_name { + data: u64, + vtable: &'static $crate::label::VTable, + } - impl ::core::fmt::Debug for $id_name { - fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { - write!(f, "{}", self.1) + impl ::std::fmt::Debug for $id_name { + fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { + (self.vtable.fmt)(self.data, f) } } $(#[$label_attr])* pub trait $label_name: 'static { /// Converts this type into an opaque, strongly-typed label. + #[inline] fn as_label(&self) -> $id_name { - let id = self.type_id(); - let label = self.as_str(); - $id_name(id, label) - } - /// Returns the [`TypeId`] used to differentiate labels. - fn type_id(&self) -> ::core::any::TypeId { - ::core::any::TypeId::of::() + // This is just machinery that lets us store the TypeId and formatter fn in the same static reference. + struct VTables(L); + impl VTables { + const VTABLE: $crate::label::VTable = $crate::label::VTable { + ty: || ::std::any::TypeId::of::(), + fmt: ::fmt, + }; + } + + let data = self.data(); + $id_name { data, vtable: &VTables::::VTABLE } } - /// Returns the representation of this label as a string literal. + /// Returns a number used to distinguish different labels of the same type. + fn data(&self) -> u64; + /// Writes debug info for a label of the current type. + /// * `data`: the result of calling [`data()`](#method.data) on an instance of this type. /// - /// 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; + /// You should not call this method directly, as it may panic for some types; + /// use [`as_label`](#method.as_label) instead. + fn fmt(data: u64, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result; } impl $label_name for $id_name { + #[inline] fn as_label(&self) -> Self { *self } - fn type_id(&self) -> ::core::any::TypeId { - self.0 + #[inline] + fn data(&self) -> u64 { + self.data } - fn as_str(&self) -> &'static str { - self.1 + #[track_caller] + fn fmt(data: u64, f: &mut ::std::fmt::Formatter) -> std::fmt::Result { + let label = stringify!($label_name); + ::std::unimplemented!("do not call `{label}::fmt` directly -- use the result of `as_label()` for formatting instead") + } + } + + impl $id_name { + /// Returns the [`TypeId`] of the label from which this ID was constructed. + /// + /// [`TypeId`]: ::std::any::TypeId + #[inline] + pub fn type_id(self) -> ::std::any::TypeId { + (self.vtable.ty)() + } + /// Returns true if this label was constructed from an instance of type `L`. + pub fn is(self) -> bool { + self.type_id() == ::std::any::TypeId::of::() } } impl $label_name for &'static str { - fn as_str(&self) -> Self { - self + fn data(&self) -> u64 { + $crate::label::STR_INTERN.intern(self) as u64 + } + fn fmt(idx: u64, f: &mut std::fmt::Formatter) -> std::fmt::Result { + let s = $crate::label::STR_INTERN + .get(idx as usize) + .ok_or(::std::fmt::Error)?; + write!(f, "{s}") } } }; From 82dc6f7766cf74d387197d6bd1c947392de64855 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 13 Aug 2022 14:44:46 -0400 Subject: [PATCH 03/19] Update derive macro --- crates/bevy_macro_utils/Cargo.toml | 1 + crates/bevy_macro_utils/src/lib.rs | 185 ++++++++++++++++++++--------- 2 files changed, 127 insertions(+), 59 deletions(-) diff --git a/crates/bevy_macro_utils/Cargo.toml b/crates/bevy_macro_utils/Cargo.toml index 4eb7aaec77381..d5fe65f750dbd 100644 --- a/crates/bevy_macro_utils/Cargo.toml +++ b/crates/bevy_macro_utils/Cargo.toml @@ -12,3 +12,4 @@ keywords = ["bevy"] toml = "0.5.8" syn = "1.0" quote = "1.0" +proc-macro2 = "1.0" diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 0af86d052a946..dbe23e85d30d1 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -9,7 +9,8 @@ pub use shape::*; pub use symbol::*; use proc_macro::TokenStream; -use quote::{quote, quote_spanned}; +use proc_macro2::{Span, TokenStream as TokenStream2}; +use quote::quote; use std::{env, path::PathBuf}; use syn::spanned::Spanned; use toml::{map::Map, Value}; @@ -105,6 +106,55 @@ impl BevyManifest { } } +/// A set of attributes defined on an item, variant, or field, +/// in the form e.g. `#[system_label(..)]`. +#[derive(Default)] +struct LabelAttrs { + ignore_fields: Option, +} + +impl LabelAttrs { + /// Parses a list of attributes. + /// + /// Ignores any that aren't of the form `#[my_label(..)]`. + /// Returns `Ok` if the iterator is empty. + pub fn new<'a>( + iter: impl IntoIterator, + attr_name: &str, + ) -> syn::Result { + let mut this = Self::default(); + for attr in iter { + // If it's not of the form `#[my_label(..)]`, skip it. + if attr.path.get_ident().as_ref().unwrap() != &attr_name { + continue; + } + + // Parse the argument/s to the attribute. + attr.parse_args_with(|input: syn::parse::ParseStream| { + loop { + syn::custom_keyword!(ignore_fields); + + let next = input.lookahead1(); + if next.peek(ignore_fields) { + let kw: ignore_fields = input.parse()?; + this.ignore_fields = Some(kw.span); + } else { + return Err(next.error()); + } + + if input.is_empty() { + break; + } + let _comma: syn::Token![,] = input.parse()?; + } + Ok(()) + })?; + } + + Ok(this) + } +} + /// Derive a label trait /// /// # Args @@ -116,23 +166,17 @@ pub fn derive_label( 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 item_attrs = match LabelAttrs::new(&input.attrs, attr_name) { + Ok(a) => a, + Err(e) => return e.into_compile_error().into(), + }; - let ident = input.ident.clone(); + derive_named_label(input, &item_attrs, trait_path, attr_name) + .unwrap_or_else(syn::Error::into_compile_error) + .into() +} - let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); +fn with_static_bound(where_clause: Option<&syn::WhereClause>) -> syn::WhereClause { let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause { where_token: Default::default(), predicates: Default::default(), @@ -140,79 +184,102 @@ pub fn derive_label( where_clause .predicates .push(syn::parse2(quote! { Self: 'static }).unwrap()); + where_clause +} + +fn derive_named_label( + input: syn::DeriveInput, + item_attrs: &LabelAttrs, + trait_path: &syn::Path, + attr_name: &str, +) -> syn::Result { + let ident = input.ident.clone(); + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + let where_clause = with_static_bound(where_clause); - let as_str = match input.data { + let (data, fmt) = match input.data { syn::Data::Struct(d) => { + let all_field_attrs = + LabelAttrs::new(d.fields.iter().flat_map(|f| &f.attrs), attr_name)?; // 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(); + if let Some(attr) = all_field_attrs.ignore_fields { + let err_msg = format!( + r#"`#[{attr_name}(ignore_fields)]` cannot be applied to fields individually: + try adding it to the struct declaration"# + ); + return Err(syn::Error::new(attr, err_msg)); } // 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 ignore_fields = item_attrs.ignore_fields.is_some(); + if d.fields.is_empty() || ignore_fields { let lit = ident.to_string(); - quote! { #lit } + let data = quote! { 0 }; + let as_str = quote! { write!(f, #lit) }; + (data, as_str) } 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(); + return Err(syn::Error::new(d.fields.span(), err_msg)); } } 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)) { + if let Some(attr) = item_attrs.ignore_fields { 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(); + return Err(syn::Error::new(attr, err_msg)); } - let arms = d.variants.iter().map(|v| { + + let mut data_arms = Vec::with_capacity(d.variants.len()); + let mut fmt_arms = Vec::with_capacity(d.variants.len()); + + for (i, v) in d.variants.iter().enumerate() { + let v_attrs = LabelAttrs::new(&v.attrs, attr_name)?; // 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 ignore_fields = v_attrs.ignore_fields.is_some(); + if v.fields.is_empty() || ignore_fields { let mut path = syn::Path::from(ident.clone()); path.segments.push(v.ident.clone().into()); + + let i = i as u64; + data_arms.push(quote! { #path { .. } => #i }); + let lit = format!("{ident}::{}", v.ident.clone()); - quote! { #path { .. } => #lit } + fmt_arms.push(quote! { #i => { write!(f, #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); } - } + return Err(syn::Error::new(v.fields.span(), err_msg)); } - }); - quote! { + } + + let data = quote! { match self { - #(#arms),* + #(#data_arms),* } - } + }; + let fmt = quote! { + match data { + #(#fmt_arms),* + _ => Err(::std::fmt::Error), + } + }; + (data, fmt) } syn::Data::Union(_) => { - return quote_spanned! { - input.span() => compile_error!("Unions cannot be used as labels."); - } - .into(); + let err_msg = format!( + "Unions cannot be used as labels, unless marked with `#[{attr_name}(intern)]`." + ); + return Err(syn::Error::new(input.span(), err_msg)); } }; - (quote! { + Ok(quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { - fn as_str(&self) -> &'static str { - #as_str + #[inline] + fn data(&self) -> u64 { + #data + } + fn fmt(data: u64, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { + #fmt } } }) - .into() } From 432ec09ad120b159dd00a687d58b3fbcdc0ea7f9 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 13 Aug 2022 15:22:35 -0400 Subject: [PATCH 04/19] Update uses of labels in the repo --- .../benches/bevy_ecs/scheduling/schedule.rs | 21 ++++++++--------- crates/bevy_app/src/app.rs | 14 +++++------ crates/bevy_ecs/src/schedule/state.rs | 23 ++++++++----------- crates/bevy_ecs/src/system/function_system.rs | 7 ++++-- 4 files changed, 31 insertions(+), 34 deletions(-) diff --git a/benches/benches/bevy_ecs/scheduling/schedule.rs b/benches/benches/bevy_ecs/scheduling/schedule.rs index 49de37079b73b..f4674b42c449f 100644 --- a/benches/benches/bevy_ecs/scheduling/schedule.rs +++ b/benches/benches/bevy_ecs/scheduling/schedule.rs @@ -64,14 +64,17 @@ 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)] - struct NumLabel(usize); + struct NumLabel(u64); #[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()) + #[inline] + fn data(&self) -> u64 { + self.0 + } + fn fmt(data: u64, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.debug_tuple("NumLabel").field(&data).finish() } } @@ -82,10 +85,6 @@ pub fn build_schedule(criterion: &mut Criterion) { // 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. - // 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] { @@ -109,12 +108,12 @@ pub fn build_schedule(criterion: &mut Criterion) { // Build a fully-connected dependency graph describing the One True Ordering. // Not particularly realistic but this can be refined later. for i in 0..graph_size { - let mut sys = empty_system.label(labels[i]).before(DummyLabel); + let mut sys = empty_system.label(NumLabel(i)).before(DummyLabel); for a in 0..i { - sys = sys.after(labels[a]); + sys = sys.after(NumLabel(a)); } for b in i + 1..graph_size { - sys = sys.before(labels[b]); + sys = sys.before(NumLabel(b)); } app.add_system(sys); } diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 7d6c027da0cdf..e733852a4222a 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -379,9 +379,9 @@ impl App { stage_label: impl StageLabel, system: impl IntoSystemDescriptor, ) -> &mut Self { - use std::any::TypeId; + let stage_label = stage_label.as_label(); assert!( - stage_label.type_id() != TypeId::of::(), + !stage_label.is::(), "add systems to a startup stage using App::add_startup_system_to_stage" ); self.schedule.add_system_to_stage(stage_label, system); @@ -414,9 +414,9 @@ impl App { stage_label: impl StageLabel, system_set: SystemSet, ) -> &mut Self { - use std::any::TypeId; + let stage_label = stage_label.as_label(); assert!( - stage_label.type_id() != TypeId::of::(), + !stage_label.is::(), "add system sets to a startup stage using App::add_startup_system_set_to_stage" ); self.schedule @@ -952,7 +952,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.as_label()), } } @@ -974,13 +974,13 @@ 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.as_label()), } } /// 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> { + pub fn get_sub_app(&self, label: L) -> Result<&App, L> { self.sub_apps .get(&label.as_label()) .map(|sub_app| &sub_app.app) diff --git a/crates/bevy_ecs/src/schedule/state.rs b/crates/bevy_ecs/src/schedule/state.rs index c08dfaa886e52..0ab452e86bcb6 100644 --- a/crates/bevy_ecs/src/schedule/state.rs +++ b/crates/bevy_ecs/src/schedule/state.rs @@ -6,7 +6,6 @@ use crate::{ system::{In, IntoChainSystem, Local, Res, ResMut, Resource}, }; use std::{ - any::TypeId, fmt::{self, Debug}, hash::Hash, }; @@ -54,20 +53,16 @@ enum ScheduledOperation { Push(T), } -#[derive(Debug, PartialEq, Eq, Clone, Hash)] -struct DriverLabel(TypeId, &'static str); -impl RunCriteriaLabel for DriverLabel { - fn type_id(&self) -> core::any::TypeId { - self.0 - } - fn as_str(&self) -> &'static str { - self.1 - } -} - +struct DriverLabel; impl DriverLabel { - fn of() -> Self { - Self(TypeId::of::(), std::any::type_name::()) + fn of() -> impl RunCriteriaLabel { + use std::marker::PhantomData; + + #[derive(RunCriteriaLabel)] + #[run_criteria_label(ignore_fields)] + struct DriverLabel(PhantomData T>); + + DriverLabel::(PhantomData) } } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 8ca47e0f4c311..ee8063fd7b20d 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -455,8 +455,11 @@ pub struct SystemTypeIdLabel(PhantomData T>); impl SystemLabel for SystemTypeIdLabel { #[inline] - fn as_str(&self) -> &'static str { - std::any::type_name::() + fn data(&self) -> u64 { + 0 + } + fn fmt(_: u64, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", std::any::type_name::()) } } From 7ac4ddb66872419f18310e53482d47cdf662efae Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 13 Aug 2022 15:33:06 -0400 Subject: [PATCH 05/19] Format generic labels --- crates/bevy_ecs/examples/derive_label.rs | 15 +++++++++++++++ crates/bevy_macro_utils/src/lib.rs | 21 ++++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/examples/derive_label.rs b/crates/bevy_ecs/examples/derive_label.rs index 573b42dc8153d..260c58e25fc54 100644 --- a/crates/bevy_ecs/examples/derive_label.rs +++ b/crates/bevy_ecs/examples/derive_label.rs @@ -18,6 +18,18 @@ fn main() { GenericLabel::::One.as_label(), GenericLabel::::One.as_label(), ); + + assert_eq!(format!("{:?}", UnitLabel.as_label()), "UnitLabel"); + assert_eq!(format!("{:?}", WeirdLabel(1).as_label()), "WeirdLabel"); + assert_eq!(format!("{:?}", WeirdLabel(2).as_label()), "WeirdLabel"); + assert_eq!( + format!("{:?}", GenericLabel::::One.as_label()), + "GenericLabel::One::" + ); + assert_eq!( + format!("{:?}", ConstGenericLabel::<21>.as_label()), + "ConstGenericLabel::<21>" + ); } #[derive(SystemLabel)] @@ -40,6 +52,9 @@ pub enum GenericLabel { Two(PhantomData), } +#[derive(SystemLabel)] +pub struct ConstGenericLabel; + // FIXME: this should be a compile_fail test /*#[derive(SystemLabel)] pub union Foo { diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index dbe23e85d30d1..215fb4bdf9876 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -271,6 +271,23 @@ fn derive_named_label( } }; + // Formatting for generics + let generics = input.generics.params.iter(); + let fmt_generics = generics + .filter_map(|p| match p { + syn::GenericParam::Type(ty) => Some({ + let ident = &ty.ident; + quote! { write!(f, "{}", ::std::any::type_name::<#ident>()) } + }), + syn::GenericParam::Const(c) => Some({ + let ident = &c.ident; + quote! { write!(f, "{:?}", { #ident }) } + }), + _ => None, + }) + .reduce(|a, b| quote! { #a?; write!(f, ", ")?; #b }) + .map(|x| quote! { write!(f, "::<")?; #x?; write!(f, ">")?; }); + Ok(quote! { impl #impl_generics #trait_path for #ident #ty_generics #where_clause { #[inline] @@ -278,7 +295,9 @@ fn derive_named_label( #data } fn fmt(data: u64, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { - #fmt + #fmt?; + #fmt_generics + Ok(()) } } }) From 913a656e33f6c84f5d3eedf10f6694d8913ba8e0 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 13 Aug 2022 15:35:20 -0400 Subject: [PATCH 06/19] Add a type for interning type-erased values --- crates/bevy_utils/src/intern.rs | 87 ++++++++++++++++++++++++++++++++- 1 file changed, 85 insertions(+), 2 deletions(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index edf9b65c0bc3f..ada246f199feb 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -1,8 +1,8 @@ -use std::hash::Hash; +use std::{any::TypeId, hash::Hash}; use parking_lot::{RwLock, RwLockReadGuard}; -use crate::FixedState; +use crate::{FixedState, StableHashMap}; type IndexSet = indexmap::IndexSet; @@ -50,3 +50,86 @@ impl Interner { RwLockReadGuard::try_map(self.0.read(), |set| set.get_index(idx)).ok() } } + +struct TypeMap(StableHashMap>); + +impl TypeMap { + pub const fn new() -> Self { + Self(StableHashMap::with_hasher(FixedState)) + } + + pub fn insert(&mut self, val: T) -> Option { + self.0.insert(TypeId::of::(), Box::new(val)) + } + pub fn get(&self) -> Option<&T> { + let val = self.0.get(&TypeId::of::())?.as_ref(); + // SAFETY: `val` was keyed with the TypeId of `T`, so we can cast it to `T`. + Some(unsafe { &*(val as *const _ as *const T) }) + } + pub fn get_mut(&mut self) -> Option<&mut T> { + let val = self.0.get_mut(&TypeId::of::())?.as_mut(); + // SAFETY: `val` was keyed with the TypeId of `T`, so we can cast it to `T`. + Some(unsafe { &mut *(val as *mut _ as *mut T) }) + } +} + +/// Data structure used to intern a set of values of any given type. +/// +/// If you just need to store a single concrete type, [`Interner`] is more efficient. +pub struct AnyInterner( + // This type-map stores instances of `IndexSet`, for any `T`. + RwLock, +); + +impl AnyInterner { + pub const fn new() -> Self { + Self(RwLock::new(TypeMap::new())) + } + + /// Interns a value, if it was not already interned in this set. + /// + /// Returns an integer used to refer to the value later on. + pub fn intern(&self, val: &L) -> usize + where + L: Clone + Hash + Eq + Send + Sync + 'static, + { + use parking_lot::RwLockUpgradableReadGuard as Guard; + + // Acquire an upgradeable read lock, since we might not have to do any writing. + let type_map = self.0.upgradable_read(); + + if let Some(set) = type_map.get::>() { + // If the value is already interned, return its index. + if let Some(idx) = set.get_index_of(val) { + return idx; + } + + // Get mutable access to the interner. + let mut type_map = Guard::upgrade(type_map); + let set = type_map.get_mut::>().unwrap(); + + // Insert a clone of the value and return its index. + let (idx, _) = set.insert_full(val.clone()); + idx + } else { + let mut type_map = Guard::upgrade(type_map); + + // Initialize the `L` interner for the first time, including `val` in it. + let mut set = IndexSet::default(); + let (idx, _) = set.insert_full(val.clone()); + let old = type_map.insert(set); + // We already checked that there is no set for type `L`, + // so let's avoid generating useless drop code for the "previous" entry. + std::mem::forget(old); + idx + } + } + + /// Gets a reference to the value with specified index. + pub fn get(&self, key: usize) -> Option> { + RwLockReadGuard::try_map(self.0.read(), |type_map| { + type_map.get::>()?.get_index(key) + }) + .ok() + } +} From 5866355276dec339b062d672a2db4b036598af74 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 13 Aug 2022 15:47:35 -0400 Subject: [PATCH 07/19] Intern complex label types --- crates/bevy_derive/src/lib.rs | 10 ++- crates/bevy_ecs/macros/src/lib.rs | 40 +++++++++--- crates/bevy_macro_utils/src/lib.rs | 98 +++++++++++++++++++++++++++--- 3 files changed, 131 insertions(+), 17 deletions(-) diff --git a/crates/bevy_derive/src/lib.rs b/crates/bevy_derive/src/lib.rs index e2088cfe04ec1..1b5be4556e9b5 100644 --- a/crates/bevy_derive/src/lib.rs +++ b/crates/bevy_derive/src/lib.rs @@ -82,8 +82,14 @@ pub fn derive_enum_variant_meta(input: TokenStream) -> TokenStream { /// Generates an impl of the `AppLabel` trait. /// -/// This works only for unit structs, or enums with only unit variants. -/// You may force a struct or variant to behave as if it were fieldless with `#[app_label(ignore_fields)]`. +/// For unit structs and enums with only unit variants, a cheap implementation can easily be created. +/// +/// More complex types must be boxed and interned +/// - opt in to this by annotating the entire item with `#[app_label(intern)]`. +/// +/// Alternatively, you may force a struct or variant to behave as if +/// it were fieldless with `#[app_label(ignore_fields)]`. +/// This is especially useful for [`PhantomData`](core::marker::PhantomData) fields. #[proc_macro_derive(AppLabel, attributes(app_label))] pub fn derive_app_label(input: TokenStream) -> TokenStream { let input = syn::parse_macro_input!(input as syn::DeriveInput); diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 98fbe27200b2e..4494d9f8cea38 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -436,8 +436,14 @@ pub fn derive_world_query(input: TokenStream) -> TokenStream { /// Generates an impl of the `SystemLabel` trait. /// -/// This works only for unit structs, or enums with only unit variants. -/// You may force a struct or variant to behave as if it were fieldless with `#[system_label(ignore_fields)]`. +/// For unit structs and enums with only unit variants, a cheap implementation can easily be created. +/// +/// More complex types must be boxed and interned +/// - opt in to this by annotating the entire item with `#[system_label(intern)]`. +/// +/// Alternatively, you may force a struct or variant to behave as if +/// it were fieldless with `#[system_label(ignore_fields)]`. +/// This is especially useful for [`PhantomData`](core::marker::PhantomData) fields. #[proc_macro_derive(SystemLabel, attributes(system_label))] pub fn derive_system_label(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); @@ -451,8 +457,14 @@ pub fn derive_system_label(input: TokenStream) -> TokenStream { /// Generates an impl of the `StageLabel` trait. /// -/// This works only for unit structs, or enums with only unit variants. -/// You may force a struct or variant to behave as if it were fieldless with `#[stage_label(ignore_fields)]`. +/// For unit structs and enums with only unit variants, a cheap implementation can easily be created. +/// +/// More complex types must be boxed and interned +/// - opt in to this by annotating the entire item with `#[stage_label(intern)]`. +/// +/// Alternatively, you may force a struct or variant to behave as if +/// it were fieldless with `#[stage_label(ignore_fields)]`. +/// This is especially useful for [`PhantomData`](core::marker::PhantomData) fields. #[proc_macro_derive(StageLabel, attributes(stage_label))] pub fn derive_stage_label(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); @@ -464,8 +476,14 @@ pub fn derive_stage_label(input: TokenStream) -> TokenStream { /// Generates an impl of the `AmbiguitySetLabel` trait. /// -/// This works only for unit structs, or enums with only unit variants. -/// You may force a struct or variant to behave as if it were fieldless with `#[ambiguity_set_label(ignore_fields)]`. +/// For unit structs and enums with only unit variants, a cheap implementation can easily be created. +/// +/// More complex types must be boxed and interned +/// - opt in to this by annotating the entire item with `#[ambiguity_set_label(intern)]`. +/// +/// Alternatively, you may force a struct or variant to behave as if +/// it were fieldless with `#[ambiguity_set_label(ignore_fields)]`. +/// This is especially useful for [`PhantomData`](core::marker::PhantomData) fields. #[proc_macro_derive(AmbiguitySetLabel, attributes(ambiguity_set_label))] pub fn derive_ambiguity_set_label(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); @@ -479,8 +497,14 @@ pub fn derive_ambiguity_set_label(input: TokenStream) -> TokenStream { /// Generates an impl of the `RunCriteriaLabel` trait. /// -/// This works only for unit structs, or enums with only unit variants. -/// You may force a struct or variant to behave as if it were fieldless with `#[run_criteria_label(ignore_fields)]`. +/// For unit structs and enums with only unit variants, a cheap implementation can easily be created. +/// +/// More complex types must be boxed and interned +/// - opt in to this by annotating the entire item with `#[run_criteria_label(intern)]`. +/// +/// Alternatively, you may force a struct or variant to behave as if +/// it were fieldless with `#[run_criteria_label(ignore_fields)]`. +/// This is especially useful for [`PhantomData`](core::marker::PhantomData) fields. #[proc_macro_derive(RunCriteriaLabel, attributes(run_criteria_label))] pub fn derive_run_criteria_label(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 215fb4bdf9876..d09f60c69d466 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -10,7 +10,7 @@ pub use symbol::*; use proc_macro::TokenStream; use proc_macro2::{Span, TokenStream as TokenStream2}; -use quote::quote; +use quote::{format_ident, quote}; use std::{env, path::PathBuf}; use syn::spanned::Spanned; use toml::{map::Map, Value}; @@ -110,6 +110,7 @@ impl BevyManifest { /// in the form e.g. `#[system_label(..)]`. #[derive(Default)] struct LabelAttrs { + intern: Option, ignore_fields: Option, } @@ -132,10 +133,14 @@ impl LabelAttrs { // Parse the argument/s to the attribute. attr.parse_args_with(|input: syn::parse::ParseStream| { loop { + syn::custom_keyword!(intern); syn::custom_keyword!(ignore_fields); let next = input.lookahead1(); - if next.peek(ignore_fields) { + if next.peek(intern) { + let kw: intern = input.parse()?; + this.intern = Some(kw.span); + } else if next.peek(ignore_fields) { let kw: ignore_fields = input.parse()?; this.ignore_fields = Some(kw.span); } else { @@ -171,9 +176,14 @@ pub fn derive_label( Err(e) => return e.into_compile_error().into(), }; - derive_named_label(input, &item_attrs, trait_path, attr_name) - .unwrap_or_else(syn::Error::into_compile_error) - .into() + // We use entirely different derives for interned and named labels. + if item_attrs.intern.is_some() { + derive_interned_label(input, trait_path, attr_name) + } else { + derive_named_label(input, &item_attrs, trait_path, attr_name) + } + .unwrap_or_else(syn::Error::into_compile_error) + .into() } fn with_static_bound(where_clause: Option<&syn::WhereClause>) -> syn::WhereClause { @@ -209,6 +219,13 @@ fn derive_named_label( ); return Err(syn::Error::new(attr, err_msg)); } + if let Some(attr) = all_field_attrs.intern { + let err_msg = format!( + r#"`#[{attr_name}(intern)]` cannot be applied to fields individually: + try adding it to the struct declaration"# + ); + return Err(syn::Error::new(attr, err_msg)); + } // Structs must either be fieldless, or explicitly ignore the fields. let ignore_fields = item_attrs.ignore_fields.is_some(); if d.fields.is_empty() || ignore_fields { @@ -217,7 +234,11 @@ fn derive_named_label( let as_str = quote! { write!(f, #lit) }; (data, as_str) } else { - let err_msg = format!("Labels cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`"); + let err_msg = format!( + r#"Simple labels cannot contain data, unless the whole type is boxed + by marking the type with `#[{attr_name}(intern)]`. + Alternatively, you can make this label behave as if it were fieldless with `#[{attr_name}(ignore_fields)]`."# + ); return Err(syn::Error::new(d.fields.span(), err_msg)); } } @@ -245,7 +266,11 @@ fn derive_named_label( let lit = format!("{ident}::{}", v.ident.clone()); fmt_arms.push(quote! { #i => { write!(f, #lit) } }); } else { - let err_msg = format!("Label variants cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`"); + let err_msg = format!( + r#"Simple labels only allow unit variants -- more complex types must be boxed + by marking the whole type with `#[{attr_name}(intern)]`. + Alternatively, you can make the variant act fieldless using `#[{attr_name}(ignore_fields)]`."# + ); return Err(syn::Error::new(v.fields.span(), err_msg)); } } @@ -302,3 +327,62 @@ fn derive_named_label( } }) } + +fn derive_interned_label( + input: syn::DeriveInput, + trait_path: &syn::Path, + _attr_name: &str, +) -> syn::Result { + let manifest = BevyManifest::default(); + + let ident = input.ident; + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + let mut where_clause = with_static_bound(where_clause); + where_clause.predicates.push( + syn::parse2(quote! { + Self: ::std::clone::Clone + ::std::cmp::Eq + ::std::hash::Hash + ::std::fmt::Debug + + ::std::marker::Send + ::std::marker::Sync + }) + .unwrap(), + ); + + let is_generic = !input.generics.params.is_empty(); + + let interner_type_path = { + let mut path = manifest.get_path("bevy_utils"); + // If the type is generic, we have to store all monomorphizations + // in the same global due to Rust restrictions. + if is_generic { + path.segments.push(format_ident!("AnyInterner").into()); + } else { + path.segments.push(format_ident!("Interner").into()); + } + path + }; + let interner_type_expr = if is_generic { + quote! { #interner_type_path } + } else { + quote! { #interner_type_path <#ident> } + }; + let guard_type_path = { + let mut path = manifest.get_path("bevy_utils"); + path.segments.push(format_ident!("InternGuard").into()); + path + }; + let interner_ident = format_ident!("{}_INTERN", ident.to_string().to_uppercase()); + + Ok(quote! { + static #interner_ident : #interner_type_expr = #interner_type_path::new(); + + impl #impl_generics #trait_path for #ident #ty_generics #where_clause { + #[inline] + fn data(&self) -> u64 { + #interner_ident .intern(self) as u64 + } + fn fmt(idx: u64, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { + let val: #guard_type_path = #interner_ident .get(idx as usize).ok_or(::std::fmt::Error)?; + ::std::fmt::Debug::fmt(&*val, f) + } + } + }) +} From 8b9e1f60e242230ed6347cbba11e136439398376 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 13 Aug 2022 15:59:41 -0400 Subject: [PATCH 08/19] Update label derive example --- crates/bevy_ecs/examples/derive_label.rs | 34 +++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/examples/derive_label.rs b/crates/bevy_ecs/examples/derive_label.rs index 260c58e25fc54..96690645db27b 100644 --- a/crates/bevy_ecs/examples/derive_label.rs +++ b/crates/bevy_ecs/examples/derive_label.rs @@ -1,4 +1,4 @@ -use std::marker::PhantomData; +use std::{fmt::Debug, hash::Hash, marker::PhantomData}; use bevy_ecs::prelude::*; @@ -30,6 +30,28 @@ fn main() { format!("{:?}", ConstGenericLabel::<21>.as_label()), "ConstGenericLabel::<21>" ); + + // Working with labels that need to be heap allocated. + let label = ComplexLabel { + people: vec!["John", "William", "Sharon"], + }; + // Convert it to a LabelId. Its type gets erased. + let id = label.as_label(); + assert_eq!( + format!("{id:?}"), + r#"ComplexLabel { people: ["John", "William", "Sharon"] }"# + ); + + // Generic heap-allocated labels. + let id = WrapLabel(1_i128).as_label(); + assert_eq!(format!("{id:?}"), "WrapLabel(1)"); + + // Different types with the same type constructor. + let id2 = WrapLabel(1_u32).as_label(); + // The debug representations are the same... + assert_eq!(format!("{id:?}"), format!("{id2:?}")); + // ...but they do not compare equal. + assert_ne!(id, id2); } #[derive(SystemLabel)] @@ -75,3 +97,13 @@ pub struct BadLabel2 { #[system_label(ignore_fields)] x: (), }*/ + +#[derive(Debug, Clone, PartialEq, Eq, Hash, SystemLabel)] +#[system_label(intern)] +pub struct ComplexLabel { + people: Vec<&'static str>, +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash, SystemLabel)] +#[system_label(intern)] +pub struct WrapLabel(T); From 11a578c603908078147aeffb07211ad0ef77a615 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 13 Aug 2022 16:12:37 -0400 Subject: [PATCH 09/19] Support downcasting interned labels --- crates/bevy_ecs/examples/derive_label.rs | 23 ++++++++++++++- crates/bevy_macro_utils/src/lib.rs | 13 +++++++++ crates/bevy_utils/src/label.rs | 37 ++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/examples/derive_label.rs b/crates/bevy_ecs/examples/derive_label.rs index 96690645db27b..7f53180d6f00d 100644 --- a/crates/bevy_ecs/examples/derive_label.rs +++ b/crates/bevy_ecs/examples/derive_label.rs @@ -41,17 +41,38 @@ fn main() { format!("{id:?}"), r#"ComplexLabel { people: ["John", "William", "Sharon"] }"# ); + // Try to downcast it back to its concrete type. + if let Some(complex_label) = id.downcast::() { + assert_eq!(complex_label.people, vec!["John", "William", "Sharon"]); + } else { + // The downcast will never fail in this example, since the label is always + // created from a value of type `ComplexLabel`. + unreachable!(); + } // Generic heap-allocated labels. let id = WrapLabel(1_i128).as_label(); assert_eq!(format!("{id:?}"), "WrapLabel(1)"); + assert!(id.downcast::>().is_none()); + if let Some(label) = id.downcast::>() { + assert_eq!(label.0, 1); + } else { + unreachable!(); + } // Different types with the same type constructor. let id2 = WrapLabel(1_u32).as_label(); // The debug representations are the same... assert_eq!(format!("{id:?}"), format!("{id2:?}")); - // ...but they do not compare equal. + // ...but they do not compare equal... assert_ne!(id, id2); + // ...nor can you downcast between monomorphizations. + assert!(id2.downcast::>().is_none()); + if let Some(label) = id2.downcast::>() { + assert_eq!(label.0, 1); + } else { + unreachable!(); + } } #[derive(SystemLabel)] diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index d09f60c69d466..3089bfef196b9 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -370,6 +370,12 @@ fn derive_interned_label( path }; let interner_ident = format_ident!("{}_INTERN", ident.to_string().to_uppercase()); + let downcast_trait_path = { + let mut path = manifest.get_path("bevy_utils"); + path.segments.push(format_ident!("label").into()); + path.segments.push(format_ident!("LabelDowncast").into()); + path + }; Ok(quote! { static #interner_ident : #interner_type_expr = #interner_type_path::new(); @@ -384,5 +390,12 @@ fn derive_interned_label( ::std::fmt::Debug::fmt(&*val, f) } } + + impl #impl_generics #downcast_trait_path for #ident #ty_generics #where_clause { + type Output = #guard_type_path <'static, Self>; + fn downcast_from(idx: u64) -> Option { + #interner_ident .get(idx as usize) + } + } }) } diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index bb25bb8f408d1..2e67311754328 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -3,6 +3,7 @@ use std::{ any::Any, hash::{Hash, Hasher}, + ops::Deref, }; use crate::Interner; @@ -49,6 +50,15 @@ where } } +/// Trait for implementors of `*Label` types that support downcasting. +pub trait LabelDowncast { + // FIXME: use "return position impl Trait in traits" when that stabilizes. + /// The type returned from [`downcast_from`](#method.downcast_from). + type Output: Deref; + /// Attempts to convert data from a label to type `Self`. Returns a reference-like type. + fn downcast_from(data: u64) -> Option; +} + #[doc(hidden)] pub struct VTable { // FIXME: When const TypeId stabilizes, inline the type instead of using a fn pointer for indirection. @@ -163,6 +173,33 @@ macro_rules! define_label { pub fn is(self) -> bool { self.type_id() == ::std::any::TypeId::of::() } + /// Attempts to downcast this label to type `L`. + /// + /// As an anti-footgun measure, the returned reference-like type is `!Send + !Sync` + /// -- often it is a mutex guard type, so it should be contained to one thread, + /// and should not be held onto for very long. + /// + /// This method is not available for all types of labels. + pub fn downcast(self) -> Option> + where + L: $label_name + $crate::label::LabelDowncast + { + // Wraps a deref type and forces it to be !Send + !Sync + struct NonSendSyncDeref(L, ::std::marker::PhantomData<*mut u8>); + impl ::std::ops::Deref for NonSendSyncDeref { + type Target = ::Target; + fn deref(&self) -> &Self::Target { + &*self.0 + } + } + + if self.is::() { + let val = L::downcast_from(self.data())?; + Some(NonSendSyncDeref(val, ::std::marker::PhantomData)) + } else { + None + } + } } impl $label_name for &'static str { From dfb3841907e1f6c23846b610fbe4fed66d026427 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 17 Aug 2022 15:39:50 -0400 Subject: [PATCH 10/19] Add a separate `Into*Label` trait for consistency --- .../benches/bevy_ecs/scheduling/schedule.rs | 4 +- crates/bevy_app/src/app.rs | 4 ++ crates/bevy_derive/src/lib.rs | 4 +- crates/bevy_ecs/macros/src/lib.rs | 10 +++-- crates/bevy_ecs/src/schedule/label.rs | 16 +++++++ crates/bevy_ecs/src/system/function_system.rs | 4 +- crates/bevy_utils/src/label.rs | 45 ++++++++++--------- 7 files changed, 57 insertions(+), 30 deletions(-) diff --git a/benches/benches/bevy_ecs/scheduling/schedule.rs b/benches/benches/bevy_ecs/scheduling/schedule.rs index f4674b42c449f..17b6e7b3f4e52 100644 --- a/benches/benches/bevy_ecs/scheduling/schedule.rs +++ b/benches/benches/bevy_ecs/scheduling/schedule.rs @@ -1,5 +1,5 @@ use bevy_app::App; -use bevy_ecs::prelude::*; +use bevy_ecs::{prelude::*, schedule::IntoSystemLabel}; use criterion::Criterion; pub fn schedule(c: &mut Criterion) { @@ -68,7 +68,7 @@ pub fn build_schedule(criterion: &mut Criterion) { #[derive(Debug, Clone, Copy, SystemLabel)] struct DummyLabel; - impl SystemLabel for NumLabel { + impl IntoSystemLabel for NumLabel { #[inline] fn data(&self) -> u64 { self.0 diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index e733852a4222a..b340d3c3ea85c 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -19,6 +19,10 @@ use bevy_utils::tracing::info_span; bevy_utils::define_label!( /// A strongly-typed class of labels used to identify an [`App`]. AppLabel, + /// Types that can be converted into [`AppLabelId`], except for `AppLabelId` itself. + /// + /// Implementing this trait automatically implements [`AppLabel`] for you. + IntoAppLabel, /// A strongly-typed identifier for an [`AppLabel`]. AppLabelId, ); diff --git a/crates/bevy_derive/src/lib.rs b/crates/bevy_derive/src/lib.rs index 1b5be4556e9b5..8b335570308b9 100644 --- a/crates/bevy_derive/src/lib.rs +++ b/crates/bevy_derive/src/lib.rs @@ -94,6 +94,8 @@ pub fn derive_enum_variant_meta(input: TokenStream) -> TokenStream { 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()); + trait_path + .segments + .push(format_ident!("IntoAppLabel").into()); derive_label(input, &trait_path, "app_label") } diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 4494d9f8cea38..565deeb7f3f30 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -451,7 +451,7 @@ pub fn derive_system_label(input: TokenStream) -> TokenStream { trait_path.segments.push(format_ident!("schedule").into()); trait_path .segments - .push(format_ident!("SystemLabel").into()); + .push(format_ident!("IntoSystemLabel").into()); derive_label(input, &trait_path, "system_label") } @@ -470,7 +470,9 @@ pub fn derive_stage_label(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); let mut trait_path = bevy_ecs_path(); trait_path.segments.push(format_ident!("schedule").into()); - trait_path.segments.push(format_ident!("StageLabel").into()); + trait_path + .segments + .push(format_ident!("IntoStageLabel").into()); derive_label(input, &trait_path, "stage_label") } @@ -491,7 +493,7 @@ pub fn derive_ambiguity_set_label(input: TokenStream) -> TokenStream { trait_path.segments.push(format_ident!("schedule").into()); trait_path .segments - .push(format_ident!("AmbiguitySetLabel").into()); + .push(format_ident!("IntoAmbiguitySetLabel").into()); derive_label(input, &trait_path, "ambiguity_set_label") } @@ -512,7 +514,7 @@ pub fn derive_run_criteria_label(input: TokenStream) -> TokenStream { trait_path.segments.push(format_ident!("schedule").into()); trait_path .segments - .push(format_ident!("RunCriteriaLabel").into()); + .push(format_ident!("IntoRunCriteriaLabel").into()); derive_label(input, &trait_path, "run_criteria_label") } diff --git a/crates/bevy_ecs/src/schedule/label.rs b/crates/bevy_ecs/src/schedule/label.rs index c9bccf8303084..21148e634d8fd 100644 --- a/crates/bevy_ecs/src/schedule/label.rs +++ b/crates/bevy_ecs/src/schedule/label.rs @@ -4,24 +4,40 @@ use bevy_utils::define_label; define_label!( /// A strongly-typed class of labels used to identify [`Stage`](crate::schedule::Stage)s. StageLabel, + /// Types that can be converted into [`StageLabelId`], except for `StageLabelId` itself. + /// + /// Implementing this trait automatically implements [`StageLabel`] for you. + IntoStageLabel, /// Strongly-typed identifier for a [`StageLabel`]. StageLabelId, ); define_label!( /// A strongly-typed class of labels used to identify [`System`](crate::system::System)s. SystemLabel, + /// Types that can be converted into [`SystemLabelId`], except for `SystemLabelId` itself. + /// + /// Implementing this trait automatically implements [`SystemLabel`] for you. + IntoSystemLabel, /// Strongly-typed identifier for a [`SystemLabel`]. SystemLabelId, ); define_label!( /// A strongly-typed class of labels used to identify sets of systems with intentionally ambiguous execution order. AmbiguitySetLabel, + /// Types that can be converted into [`AmbiguitySetLabelId`], except for `AmbiguitySetLabelId` itself. + /// + /// Implementing this trait automatically implements [`AmbiguitySetLabel`] for you. + IntoAmbiguitySetLabel, /// Strongly-typed identifier for an [`AmbiguitySetLabel`]. AmbiguitySetLabelId, ); define_label!( /// A strongly-typed class of labels used to identify [run criteria](crate::schedule::RunCriteria). RunCriteriaLabel, + /// Types that can be converted into [`RunCriteriaLabelId`], except for `RunCriteriaLabelId` itself. + /// + /// Implementing this trait automatically implements [`RunCriteriaLabel`] for you. + IntoRunCriteriaLabel, /// Strongly-typed identifier for a [`RunCriteriaLabel`]. RunCriteriaLabelId, ); diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index ee8063fd7b20d..6dfe3fb34adf6 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -4,7 +4,7 @@ use crate::{ component::ComponentId, prelude::FromWorld, query::{Access, FilteredAccessSet}, - schedule::{SystemLabel, SystemLabelId}, + schedule::{IntoSystemLabel, SystemLabel, SystemLabelId}, system::{ check_system_change_tick, ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch, SystemParamItem, SystemParamState, @@ -453,7 +453,7 @@ where /// A [`SystemLabel`] that was automatically generated for a system on the basis of its `TypeId`. pub struct SystemTypeIdLabel(PhantomData T>); -impl SystemLabel for SystemTypeIdLabel { +impl IntoSystemLabel for SystemTypeIdLabel { #[inline] fn data(&self) -> u64 { 0 diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 2e67311754328..82711f121c7aa 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -92,6 +92,10 @@ pub static STR_INTERN: Interner<&str> = Interner::new(); /// define_label!( /// /// A class of labels. /// MyNewLabelTrait, +/// /// Types that can be converted to [`MyNewLabelId`], except for `MyNewLabelId` itself. +/// /// +/// /// Implementing this trait automatically implements [`MyNewLabelTrait`] for you. +/// IntoMyNewLabel, /// /// Identifies a value that implements `MyNewLabelTrait`. /// MyNewLabelId, /// ); @@ -102,6 +106,9 @@ macro_rules! define_label { $(#[$label_attr:meta])* $label_name:ident, + $(#[$into_attr:meta])* + $into_label:ident, + $(#[$id_attr:meta])* $id_name:ident $(,)? ) => { @@ -121,28 +128,33 @@ macro_rules! define_label { $(#[$label_attr])* pub trait $label_name: 'static { /// Converts this type into an opaque, strongly-typed label. + fn as_label(&self) -> $id_name; + } + + $(#[$into_attr])* + pub trait $into_label: 'static { + /// Returns a number used to distinguish different labels of the same type. + fn data(&self) -> u64; + /// Writes debug info for a label of the current type. + /// * `data`: the result of calling [`data()`](#method.data) on an instance of this type. + fn fmt(data: u64, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result; + } + + impl $label_name for T { #[inline] fn as_label(&self) -> $id_name { // This is just machinery that lets us store the TypeId and formatter fn in the same static reference. struct VTables(L); - impl VTables { + impl VTables { const VTABLE: $crate::label::VTable = $crate::label::VTable { ty: || ::std::any::TypeId::of::(), - fmt: ::fmt, + fmt: ::fmt, }; } let data = self.data(); $id_name { data, vtable: &VTables::::VTABLE } } - /// Returns a number used to distinguish different labels of the same type. - fn data(&self) -> u64; - /// Writes debug info for a label of the current type. - /// * `data`: the result of calling [`data()`](#method.data) on an instance of this type. - /// - /// You should not call this method directly, as it may panic for some types; - /// use [`as_label`](#method.as_label) instead. - fn fmt(data: u64, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result; } impl $label_name for $id_name { @@ -150,15 +162,6 @@ macro_rules! define_label { fn as_label(&self) -> Self { *self } - #[inline] - fn data(&self) -> u64 { - self.data - } - #[track_caller] - fn fmt(data: u64, f: &mut ::std::fmt::Formatter) -> std::fmt::Result { - let label = stringify!($label_name); - ::std::unimplemented!("do not call `{label}::fmt` directly -- use the result of `as_label()` for formatting instead") - } } impl $id_name { @@ -194,7 +197,7 @@ macro_rules! define_label { } if self.is::() { - let val = L::downcast_from(self.data())?; + let val = L::downcast_from(self.data)?; Some(NonSendSyncDeref(val, ::std::marker::PhantomData)) } else { None @@ -202,7 +205,7 @@ macro_rules! define_label { } } - impl $label_name for &'static str { + impl $into_label for &'static str { fn data(&self) -> u64 { $crate::label::STR_INTERN.intern(self) as u64 } From 8b0db0d568c727dbbc1dd55b738518c0d0afdade Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 3 Sep 2022 15:03:38 -0400 Subject: [PATCH 11/19] Remove downcasting support --- crates/bevy_ecs/examples/derive_label.rs | 23 +-------------- crates/bevy_macro_utils/src/lib.rs | 13 --------- crates/bevy_utils/src/label.rs | 37 ------------------------ 3 files changed, 1 insertion(+), 72 deletions(-) diff --git a/crates/bevy_ecs/examples/derive_label.rs b/crates/bevy_ecs/examples/derive_label.rs index 7f53180d6f00d..96690645db27b 100644 --- a/crates/bevy_ecs/examples/derive_label.rs +++ b/crates/bevy_ecs/examples/derive_label.rs @@ -41,38 +41,17 @@ fn main() { format!("{id:?}"), r#"ComplexLabel { people: ["John", "William", "Sharon"] }"# ); - // Try to downcast it back to its concrete type. - if let Some(complex_label) = id.downcast::() { - assert_eq!(complex_label.people, vec!["John", "William", "Sharon"]); - } else { - // The downcast will never fail in this example, since the label is always - // created from a value of type `ComplexLabel`. - unreachable!(); - } // Generic heap-allocated labels. let id = WrapLabel(1_i128).as_label(); assert_eq!(format!("{id:?}"), "WrapLabel(1)"); - assert!(id.downcast::>().is_none()); - if let Some(label) = id.downcast::>() { - assert_eq!(label.0, 1); - } else { - unreachable!(); - } // Different types with the same type constructor. let id2 = WrapLabel(1_u32).as_label(); // The debug representations are the same... assert_eq!(format!("{id:?}"), format!("{id2:?}")); - // ...but they do not compare equal... + // ...but they do not compare equal. assert_ne!(id, id2); - // ...nor can you downcast between monomorphizations. - assert!(id2.downcast::>().is_none()); - if let Some(label) = id2.downcast::>() { - assert_eq!(label.0, 1); - } else { - unreachable!(); - } } #[derive(SystemLabel)] diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 3089bfef196b9..d09f60c69d466 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -370,12 +370,6 @@ fn derive_interned_label( path }; let interner_ident = format_ident!("{}_INTERN", ident.to_string().to_uppercase()); - let downcast_trait_path = { - let mut path = manifest.get_path("bevy_utils"); - path.segments.push(format_ident!("label").into()); - path.segments.push(format_ident!("LabelDowncast").into()); - path - }; Ok(quote! { static #interner_ident : #interner_type_expr = #interner_type_path::new(); @@ -390,12 +384,5 @@ fn derive_interned_label( ::std::fmt::Debug::fmt(&*val, f) } } - - impl #impl_generics #downcast_trait_path for #ident #ty_generics #where_clause { - type Output = #guard_type_path <'static, Self>; - fn downcast_from(idx: u64) -> Option { - #interner_ident .get(idx as usize) - } - } }) } diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 82711f121c7aa..c0601106ff087 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -3,7 +3,6 @@ use std::{ any::Any, hash::{Hash, Hasher}, - ops::Deref, }; use crate::Interner; @@ -50,15 +49,6 @@ where } } -/// Trait for implementors of `*Label` types that support downcasting. -pub trait LabelDowncast { - // FIXME: use "return position impl Trait in traits" when that stabilizes. - /// The type returned from [`downcast_from`](#method.downcast_from). - type Output: Deref; - /// Attempts to convert data from a label to type `Self`. Returns a reference-like type. - fn downcast_from(data: u64) -> Option; -} - #[doc(hidden)] pub struct VTable { // FIXME: When const TypeId stabilizes, inline the type instead of using a fn pointer for indirection. @@ -176,33 +166,6 @@ macro_rules! define_label { pub fn is(self) -> bool { self.type_id() == ::std::any::TypeId::of::() } - /// Attempts to downcast this label to type `L`. - /// - /// As an anti-footgun measure, the returned reference-like type is `!Send + !Sync` - /// -- often it is a mutex guard type, so it should be contained to one thread, - /// and should not be held onto for very long. - /// - /// This method is not available for all types of labels. - pub fn downcast(self) -> Option> - where - L: $label_name + $crate::label::LabelDowncast - { - // Wraps a deref type and forces it to be !Send + !Sync - struct NonSendSyncDeref(L, ::std::marker::PhantomData<*mut u8>); - impl ::std::ops::Deref for NonSendSyncDeref { - type Target = ::Target; - fn deref(&self) -> &Self::Target { - &*self.0 - } - } - - if self.is::() { - let val = L::downcast_from(self.data)?; - Some(NonSendSyncDeref(val, ::std::marker::PhantomData)) - } else { - None - } - } } impl $into_label for &'static str { From 5483d3123f4f04e62d1b5dfa5ddb73748ad3e71c Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 3 Sep 2022 15:40:08 -0400 Subject: [PATCH 12/19] Clarify comments for `Into*Label` traits --- crates/bevy_app/src/app.rs | 2 +- crates/bevy_ecs/src/schedule/label.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index b340d3c3ea85c..32bd09a087b02 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -21,7 +21,7 @@ bevy_utils::define_label!( AppLabel, /// Types that can be converted into [`AppLabelId`], except for `AppLabelId` itself. /// - /// Implementing this trait automatically implements [`AppLabel`] for you. + /// Implementing this trait automatically implements [`AppLabel`] due to a blanket implementation. IntoAppLabel, /// A strongly-typed identifier for an [`AppLabel`]. AppLabelId, diff --git a/crates/bevy_ecs/src/schedule/label.rs b/crates/bevy_ecs/src/schedule/label.rs index 21148e634d8fd..89256b95a6803 100644 --- a/crates/bevy_ecs/src/schedule/label.rs +++ b/crates/bevy_ecs/src/schedule/label.rs @@ -6,7 +6,7 @@ define_label!( StageLabel, /// Types that can be converted into [`StageLabelId`], except for `StageLabelId` itself. /// - /// Implementing this trait automatically implements [`StageLabel`] for you. + /// Implementing this trait automatically implements [`StageLabel`] due to a blanket implementation. IntoStageLabel, /// Strongly-typed identifier for a [`StageLabel`]. StageLabelId, @@ -16,7 +16,7 @@ define_label!( SystemLabel, /// Types that can be converted into [`SystemLabelId`], except for `SystemLabelId` itself. /// - /// Implementing this trait automatically implements [`SystemLabel`] for you. + /// Implementing this trait automatically implements [`SystemLabel`] due to a blanket implementation. IntoSystemLabel, /// Strongly-typed identifier for a [`SystemLabel`]. SystemLabelId, @@ -26,7 +26,7 @@ define_label!( AmbiguitySetLabel, /// Types that can be converted into [`AmbiguitySetLabelId`], except for `AmbiguitySetLabelId` itself. /// - /// Implementing this trait automatically implements [`AmbiguitySetLabel`] for you. + /// Implementing this trait automatically implements [`AmbiguitySetLabel`] due to a blanket implementation. IntoAmbiguitySetLabel, /// Strongly-typed identifier for an [`AmbiguitySetLabel`]. AmbiguitySetLabelId, @@ -36,7 +36,7 @@ define_label!( RunCriteriaLabel, /// Types that can be converted into [`RunCriteriaLabelId`], except for `RunCriteriaLabelId` itself. /// - /// Implementing this trait automatically implements [`RunCriteriaLabel`] for you. + /// Implementing this trait automatically implements [`RunCriteriaLabel`] due to a blanket implementation. IntoRunCriteriaLabel, /// Strongly-typed identifier for a [`RunCriteriaLabel`]. RunCriteriaLabelId, From 79ddaaf5ce00a317080fd70dbff3ecb71826d215 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 3 Sep 2022 15:50:01 -0400 Subject: [PATCH 13/19] Clarify the vtable magic code --- crates/bevy_utils/src/label.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index c0601106ff087..ef5ca5cc76e6c 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -133,9 +133,11 @@ macro_rules! define_label { impl $label_name for T { #[inline] fn as_label(&self) -> $id_name { - // This is just machinery that lets us store the TypeId and formatter fn in the same static reference. + // This type never gets created, it is only used to declare an associated const that uses `T`. struct VTables(L); impl VTables { + // Store the `TypeId` and formatter fn in the same static, + // so they can both be referred to using a single pointer. const VTABLE: $crate::label::VTable = $crate::label::VTable { ty: || ::std::any::TypeId::of::(), fmt: ::fmt, From 5a92fbaf18d7f1bd44c2b954bbf44fbd75bd9edf Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Sat, 3 Sep 2022 16:21:21 -0400 Subject: [PATCH 14/19] Fix a merge error --- crates/bevy_app/src/app.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index eb23062321b63..92abc4f6b607c 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -432,7 +432,7 @@ impl App { ) -> &mut Self { let stage_label = stage_label.as_label(); assert!( - stage_label.is::(), + !stage_label.is::(), "use `add_startup_system_set_to_stage` instead of `add_system_set_to_stage` to add system sets to a StartupStage" ); self.schedule From 871e299322110cb6f640e8121bbef0e0b32269c4 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Mon, 19 Sep 2022 17:32:57 -0400 Subject: [PATCH 15/19] Link to a rust tracking issue --- crates/bevy_utils/src/label.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index ef5ca5cc76e6c..231307f8538f3 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -52,6 +52,7 @@ where #[doc(hidden)] pub struct VTable { // FIXME: When const TypeId stabilizes, inline the type instead of using a fn pointer for indirection. + // See https://github.com/rust-lang/rust/issues/77125 pub ty: fn() -> ::std::any::TypeId, pub fmt: fn(u64, &mut ::std::fmt::Formatter) -> ::std::fmt::Result, } From d35eaee065643452384dc94d88708ab9daa32ba5 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Mon, 19 Sep 2022 17:36:23 -0400 Subject: [PATCH 16/19] Use a better style for optional plurals --- crates/bevy_macro_utils/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index d09f60c69d466..f94d78f0ecf06 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -130,7 +130,7 @@ impl LabelAttrs { continue; } - // Parse the argument/s to the attribute. + // Parse the argument(s) to the attribute. attr.parse_args_with(|input: syn::parse::ParseStream| { loop { syn::custom_keyword!(intern); From dcf7db4e4a00e4ab9b2dfd502fa0bc96a43a8e52 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Mon, 19 Sep 2022 18:06:17 -0400 Subject: [PATCH 17/19] Add module-level docs about interning --- crates/bevy_utils/src/intern.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index ada246f199feb..9ec17919cd980 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -1,3 +1,14 @@ +//! Provides types used to statically intern immutable values. +//! +//! Interning is a pattern used to save memory by deduplicating identical values, +//! speed up code by shrinking the stack size of large types, +//! and make comparisons for any type as fast as integers. +//! +//! Interned values must be stored in a global resource, which has two major consequences: +//! * Creating and accessing interning values requires thread synchronization, +//! while passing and comparing references is nearly free, no matter the wrapped type. +//! * A type can only be interned if it is `Send + Sync`. + use std::{any::TypeId, hash::Hash}; use parking_lot::{RwLock, RwLockReadGuard}; @@ -7,6 +18,8 @@ use crate::{FixedState, StableHashMap}; type IndexSet = indexmap::IndexSet; /// A data structure used to intern a set of values of a specific type. +/// For details on interning, see [the module level docs](self). +/// /// To store multiple distinct types, or generic types, try [`AnyInterner`]. pub struct Interner( // The `IndexSet` is a hash set that preserves ordering as long as @@ -74,6 +87,7 @@ impl TypeMap { } /// Data structure used to intern a set of values of any given type. +/// For details on interning, see [the module level docs](self). /// /// If you just need to store a single concrete type, [`Interner`] is more efficient. pub struct AnyInterner( From f9a2cd194112bc4aeb65f0d4596c453cbad8bd96 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Mon, 19 Sep 2022 18:09:30 -0400 Subject: [PATCH 18/19] Make some implicit bounds explicit --- crates/bevy_utils/src/intern.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_utils/src/intern.rs b/crates/bevy_utils/src/intern.rs index 9ec17919cd980..1881f9fc71b62 100644 --- a/crates/bevy_utils/src/intern.rs +++ b/crates/bevy_utils/src/intern.rs @@ -21,7 +21,7 @@ type IndexSet = indexmap::IndexSet; /// For details on interning, see [the module level docs](self). /// /// To store multiple distinct types, or generic types, try [`AnyInterner`]. -pub struct Interner( +pub struct Interner( // The `IndexSet` is a hash set that preserves ordering as long as // you don't remove items (which we don't). // This allows us to have O(~1) hashing and map each entry to a stable index. @@ -33,7 +33,7 @@ pub struct Interner( /// Will hold a lock on the interner until this guard gets dropped. pub type InternGuard<'a, L> = parking_lot::MappedRwLockReadGuard<'a, L>; -impl Interner { +impl Interner { pub const fn new() -> Self { Self(RwLock::new(IndexSet::with_hasher(FixedState))) } From f8ca6564634b77142b8e0813de035746b8a0751a Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Mon, 19 Sep 2022 18:25:35 -0400 Subject: [PATCH 19/19] Make `intern` module public --- crates/bevy_macro_utils/src/lib.rs | 2 ++ crates/bevy_utils/src/label.rs | 2 +- crates/bevy_utils/src/lib.rs | 3 +-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index f94d78f0ecf06..a9ef4c95697a3 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -350,6 +350,7 @@ fn derive_interned_label( let interner_type_path = { let mut path = manifest.get_path("bevy_utils"); + path.segments.push(format_ident!("intern").into()); // If the type is generic, we have to store all monomorphizations // in the same global due to Rust restrictions. if is_generic { @@ -366,6 +367,7 @@ fn derive_interned_label( }; let guard_type_path = { let mut path = manifest.get_path("bevy_utils"); + path.segments.push(format_ident!("intern").into()); path.segments.push(format_ident!("InternGuard").into()); path }; diff --git a/crates/bevy_utils/src/label.rs b/crates/bevy_utils/src/label.rs index 231307f8538f3..975f2d76a7a50 100644 --- a/crates/bevy_utils/src/label.rs +++ b/crates/bevy_utils/src/label.rs @@ -5,7 +5,7 @@ use std::{ hash::{Hash, Hasher}, }; -use crate::Interner; +use crate::intern::Interner; pub trait DynEq: Any { fn as_any(&self) -> &dyn Any; diff --git a/crates/bevy_utils/src/lib.rs b/crates/bevy_utils/src/lib.rs index 22ed9a3d9066c..8be81cbe6459b 100644 --- a/crates/bevy_utils/src/lib.rs +++ b/crates/bevy_utils/src/lib.rs @@ -6,18 +6,17 @@ pub mod futures; pub mod label; mod short_names; pub use short_names::get_short_name; +pub mod intern; pub mod synccell; mod default; mod float_ord; -mod intern; pub use ahash::AHasher; pub use default::default; pub use float_ord::*; pub use hashbrown; pub use instant::{Duration, Instant}; -pub use intern::*; pub use tracing; pub use uuid::Uuid;