From c3a88b7bf2de9b2a3856f7a2f4f2e91791c9e4db Mon Sep 17 00:00:00 2001 From: Nicola Papale Date: Sun, 20 Aug 2023 09:38:28 +0200 Subject: [PATCH] Make get_short_name return Cow instead of String --- crates/bevy_ecs/src/schedule/schedule.rs | 51 ++++++++------ crates/bevy_reflect/src/lib.rs | 2 +- crates/bevy_reflect/src/type_registry.rs | 8 ++- crates/bevy_utils/src/lib.rs | 2 +- crates/bevy_utils/src/short_names.rs | 85 +++++++++++++----------- 5 files changed, 84 insertions(+), 64 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index cb9ab0a9783350..9d52fc8720ee39 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -1,4 +1,5 @@ use std::{ + borrow::Cow, fmt::{Debug, Write}, result::Result, }; @@ -23,6 +24,8 @@ use crate::{ world::World, }; +type CowStr = Cow<'static, str>; + /// Resource that stores [`Schedule`]s mapped to [`ScheduleLabel`]s. #[derive(Default, Resource)] pub struct Schedules { @@ -1241,18 +1244,18 @@ enum ReportCycles { // methods for reporting errors impl ScheduleGraph { - fn get_node_name(&self, id: &NodeId) -> String { - let mut name = match id { + fn get_node_name(&self, id: &NodeId) -> CowStr { + let mut name: Cow<_> = match id { NodeId::System(_) => { - let name = self.systems[id.index()].get().unwrap().name().to_string(); + let name = self.systems[id.index()].get().unwrap().name(); if self.settings.report_sets { let sets = self.names_of_sets_containing_node(id); if sets.is_empty() { name } else if sets.len() == 1 { - format!("{name} (in set {})", sets[0]) + format!("{name} (in set {})", sets[0]).into() } else { - format!("{name} (in sets {})", sets.join(", ")) + format!("{name} (in sets {})", sets.join(", ")).into() } } else { name @@ -1261,14 +1264,17 @@ impl ScheduleGraph { NodeId::Set(_) => { let set = &self.system_sets[id.index()]; if set.is_anonymous() { - self.anonymous_set_name(id) + self.anonymous_set_name(id).into() } else { - set.name() + set.name().into() } } }; if self.settings.use_shortnames { - name = bevy_utils::get_short_name(&name); + name = match name { + Cow::Borrowed(borrowed) => bevy_utils::get_short_name(borrowed), + Cow::Owned(string) => Cow::Owned(bevy_utils::get_short_name(&string).to_string()), + }; } name } @@ -1280,7 +1286,12 @@ impl ScheduleGraph { .graph .edges_directed(*id, Direction::Outgoing) .map(|(_, member_id, _)| self.get_node_name(&member_id)) - .reduce(|a, b| format!("{a}, {b}")) + .reduce(|mut a, b| { + let a_mut = a.to_mut(); + *a_mut += ", "; + *a_mut += &b; + a + }) .unwrap_or_default() ) } @@ -1463,7 +1474,7 @@ impl ScheduleGraph { } } - fn names_of_sets_containing_node(&self, id: &NodeId) -> Vec { + fn names_of_sets_containing_node(&self, id: &NodeId) -> Vec { let mut sets = HashSet::new(); self.traverse_sets_containing_node(*id, &mut |set_id| { !self.system_sets[set_id.index()].is_system_type() && sets.insert(set_id) @@ -1482,8 +1493,8 @@ impl ScheduleGraph { #[non_exhaustive] pub enum ScheduleBuildError { /// A system set contains itself. - #[error("`{0:?}` contains itself.")] - HierarchyLoop(String), + #[error("`{0}` contains itself.")] + HierarchyLoop(CowStr), /// The hierarchy of system sets contains a cycle. #[error("System set hierarchy contains cycle(s).")] HierarchyCycle, @@ -1493,20 +1504,20 @@ pub enum ScheduleBuildError { #[error("System set hierarchy contains redundant edges.")] HierarchyRedundancy, /// A system (set) has been told to run before itself. - #[error("`{0:?}` depends on itself.")] - DependencyLoop(String), + #[error("`{0}` depends on itself.")] + DependencyLoop(CowStr), /// The dependency graph contains a cycle. #[error("System dependencies contain cycle(s).")] DependencyCycle, /// Tried to order a system (set) relative to a system set it belongs to. - #[error("`{0:?}` and `{1:?}` have both `in_set` and `before`-`after` relationships (these might be transitive). This combination is unsolvable as a system cannot run before or after a set it belongs to.")] - CrossDependency(String, String), + #[error("`{0}` and `{1}` have both `in_set` and `before`-`after` relationships (these might be transitive). This combination is unsolvable as a system cannot run before or after a set it belongs to.")] + CrossDependency(CowStr, CowStr), /// Tried to order system sets that share systems. - #[error("`{0:?}` and `{1:?}` have a `before`-`after` relationship (which may be transitive) but share systems.")] - SetsHaveOrderButIntersect(String, String), + #[error("`{0}` and `{1}` have a `before`-`after` relationship (which may be transitive) but share systems.")] + SetsHaveOrderButIntersect(CowStr, CowStr), /// Tried to order a system (set) relative to all instances of some system function. - #[error("Tried to order against `fn {0:?}` in a schedule that has more than one `{0:?}` instance. `fn {0:?}` is a `SystemTypeSet` and cannot be used for ordering if ambiguous. Use a different set without this restriction.")] - SystemTypeSetAmbiguity(String), + #[error("Tried to order against `fn {0}` in a schedule that has more than one `{0}` instance. `fn {0}` is a `SystemTypeSet` and cannot be used for ordering if ambiguous. Use a different set without this restriction.")] + SystemTypeSetAmbiguity(CowStr), /// Systems with conflicting access have indeterminate run order. /// /// This error is disabled by default, but can be opted-in using [`ScheduleBuildSettings`]. diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 1290ebf48cd163..68512e63a58ed2 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -1914,7 +1914,7 @@ bevy_reflect::tests::should_reflect_debug::Test { fn short_type_path() -> &'static str { static CELL: GenericTypePathCell = GenericTypePathCell::new(); CELL.get_or_insert::(|| { - bevy_utils::get_short_name(std::any::type_name::()) + bevy_utils::get_short_name(std::any::type_name::()).to_string() }) } diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index aeaba79a2f6d53..86bac15d3c41d8 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -1,10 +1,12 @@ -use crate::{serde::Serializable, Reflect, TypeInfo, Typed}; +use std::{any::TypeId, borrow::Cow, fmt::Debug, sync::Arc}; + use bevy_ptr::{Ptr, PtrMut}; use bevy_utils::{HashMap, HashSet}; use downcast_rs::{impl_downcast, Downcast}; use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use serde::Deserialize; -use std::{any::TypeId, fmt::Debug, sync::Arc}; + +use crate::{serde::Serializable, Reflect, TypeInfo, Typed}; /// A registry of [reflected] types. /// @@ -303,7 +305,7 @@ impl TypeRegistryArc { /// [short name]: bevy_utils::get_short_name /// [crate-level documentation]: crate pub struct TypeRegistration { - short_name: String, + short_name: Cow<'static, str>, data: HashMap>, type_info: &'static TypeInfo, } diff --git a/crates/bevy_utils/src/lib.rs b/crates/bevy_utils/src/lib.rs index 3dc32e81d89045..d31312af24aeb8 100644 --- a/crates/bevy_utils/src/lib.rs +++ b/crates/bevy_utils/src/lib.rs @@ -14,7 +14,7 @@ pub mod prelude { pub mod futures; pub mod label; mod short_names; -pub use short_names::get_short_name; +pub use short_names::{get_short_name, DisplayShortName}; pub mod synccell; pub mod syncunsafecell; diff --git a/crates/bevy_utils/src/short_names.rs b/crates/bevy_utils/src/short_names.rs index d8abbdcf1e6359..dc8eaf6fc3af3b 100644 --- a/crates/bevy_utils/src/short_names.rs +++ b/crates/bevy_utils/src/short_names.rs @@ -1,65 +1,72 @@ +use std::{borrow::Cow, fmt, str}; + +const SPECIAL_TYPE_CHARS: [u8; 9] = *b" <>()[],;"; /// Shortens a type name to remove all module paths. /// /// The short name of a type is its full name as returned by /// [`std::any::type_name`], but with the prefix of all paths removed. For /// example, the short name of `alloc::vec::Vec>` /// would be `Vec>`. -pub fn get_short_name(full_name: &str) -> String { +pub fn get_short_name(full_name: &str) -> Cow { // Generics result in nested paths within <..> blocks. // Consider "bevy_render::camera::camera::extract_cameras". // To tackle this, we parse the string from left to right, collapsing as we go. - let mut index: usize = 0; - let end_of_string = full_name.len(); - let mut parsed_name = String::new(); - - while index < end_of_string { - let rest_of_string = full_name.get(index..end_of_string).unwrap_or_default(); + let mut remaining = full_name.as_bytes(); + let mut parsed_name = Vec::new(); + let mut complex_type = false; + loop { // Collapse everything up to the next special character, // then skip over it - if let Some(special_character_index) = rest_of_string.find(|c: char| { - (c == ' ') - || (c == '<') - || (c == '>') - || (c == '(') - || (c == ')') - || (c == '[') - || (c == ']') - || (c == ',') - || (c == ';') - }) { - let segment_to_collapse = rest_of_string - .get(0..special_character_index) - .unwrap_or_default(); - parsed_name += collapse_type_name(segment_to_collapse); - // Insert the special character - let special_character = - &rest_of_string[special_character_index..=special_character_index]; - parsed_name.push_str(special_character); - - match special_character { - ">" | ")" | "]" - if rest_of_string[special_character_index + 1..].starts_with("::") => - { - parsed_name.push_str("::"); + let is_special = |c| SPECIAL_TYPE_CHARS.contains(c); + if let Some(next_special_index) = remaining.iter().position(is_special) { + complex_type = true; + if parsed_name.is_empty() { + parsed_name.reserve(remaining.len()); + } + let (pre_special, post_special) = remaining.split_at(next_special_index + 1); + parsed_name.extend_from_slice(collapse_type_name(pre_special)); + match pre_special.last().unwrap() { + b'>' | b')' | b']' if post_special.get(..2) == Some(b"::") => { + parsed_name.extend_from_slice(b"::"); // Move the index past the "::" - index += special_character_index + 3; + remaining = &post_special[2..]; } // Move the index just past the special character - _ => index += special_character_index + 1, + _ => remaining = post_special, } + } else if !complex_type { + let collapsed = collapse_type_name(remaining); + // SAFETY: We only split on ASCII characters, and the input is valid UTF8, since + // it was a &str + let str = unsafe { str::from_utf8_unchecked(collapsed) }; + return Cow::Borrowed(str); } else { // If there are no special characters left, we're done! - parsed_name += collapse_type_name(rest_of_string); - index = end_of_string; + parsed_name.extend_from_slice(collapse_type_name(remaining)); + // SAFETY: see above + let utf8_name = unsafe { String::from_utf8_unchecked(parsed_name) }; + return Cow::Owned(utf8_name); } } - parsed_name +} + +/// Wrapper around `AsRef` that uses the [`get_short_name`] format when +/// displayed. +pub struct DisplayShortName<'a, T: AsRef>(pub &'a T); + +impl> fmt::Display for DisplayShortName<'_, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let as_short_name = get_short_name(self.0.as_ref()); + write!(f, "{as_short_name}") + } } #[inline(always)] -fn collapse_type_name(string: &str) -> &str { - string.split("::").last().unwrap() +fn collapse_type_name(string: &[u8]) -> &[u8] { + let find = |(index, window)| (window == b"::").then_some(index + 2); + let split_index = string.windows(2).enumerate().rev().find_map(find); + &string[split_index.unwrap_or(0)..] } #[cfg(test)]