Remove allocation in get_short_name (#15294)

`ShortName` is lazily evaluated and does not allocate, instead providing
`Display` and `Debug` implementations which write directly to a
formatter using the original algorithm. When using `ShortName` in format
strings (`panic`, `dbg`, `format`, etc.) you can directly use the
`ShortName` type. If you require a `String`, simply call
`ShortName(...).to_string()`.

# Objective

- Remove the requirement for allocation when using `get_short_name`

## Solution

- Added new type `ShortName` which wraps a name and provides its own
`Debug` and `Display` implementations, using the original
`get_short_name` algorithm without the need for allocating.
- Removed `get_short_name`, as `ShortName(...)` is more performant and
ergonomic.
- Added `ShortName::of::<T>` method to streamline the common use-case
for name shortening.

## Testing

- CI

## Migration Guide

### For `format!`, `dbg!`, `panic!`, etc.

```rust
// Before
panic!("{} is too short!", get_short_name(name));

// After
panic!("{} is too short!", ShortName(name));
```

### Need a `String` Value

```rust
// Before
let short: String = get_short_name(name);

// After
let short: String = ShortName(name).to_string();
```

## Notes

`ShortName` lazily evaluates, and directly writes to a formatter via
`Debug` and `Display`, which removes the need to allocate a `String`
when printing a shortened type name. Because the implementation has been
moved into the `fmt` method, repeated printing of the `ShortName` type
may be less performant than converting it into a `String`. However, no
instances of this are present in Bevy, and the user can get the original
behaviour by calling `.to_string()` at no extra cost.

---------

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
This commit is contained in:
Zachary Harrold 2024-09-20 01:34:03 +10:00 committed by GitHub
parent 3a66d88c83
commit fcfa60844a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 134 additions and 79 deletions

View file

@ -4,7 +4,7 @@ use crate::{
}; };
use bevy_ecs::prelude::*; use bevy_ecs::prelude::*;
use bevy_reflect::{std_traits::ReflectDefault, Reflect, TypePath}; use bevy_reflect::{std_traits::ReflectDefault, Reflect, TypePath};
use bevy_utils::get_short_name; use bevy_utils::ShortName;
use crossbeam_channel::{Receiver, Sender}; use crossbeam_channel::{Receiver, Sender};
use std::{ use std::{
any::TypeId, any::TypeId,
@ -206,7 +206,7 @@ impl<A: Asset> Default for Handle<A> {
impl<A: Asset> std::fmt::Debug for Handle<A> { impl<A: Asset> std::fmt::Debug for Handle<A> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let name = get_short_name(std::any::type_name::<A>()); let name = ShortName::of::<A>();
match self { match self {
Handle::Strong(handle) => { Handle::Strong(handle) => {
write!( write!(

View file

@ -1608,7 +1608,7 @@ impl ScheduleGraph {
} }
}; };
if self.settings.use_shortnames { if self.settings.use_shortnames {
name = bevy_utils::get_short_name(&name); name = bevy_utils::ShortName(&name).to_string();
} }
name name
} }

View file

@ -4,7 +4,7 @@ use std::marker::PhantomData;
use crate::Parent; use crate::Parent;
use bevy_ecs::prelude::*; use bevy_ecs::prelude::*;
#[cfg(feature = "bevy_app")] #[cfg(feature = "bevy_app")]
use bevy_utils::{get_short_name, HashSet}; use bevy_utils::{HashSet, ShortName};
/// When enabled, runs [`check_hierarchy_component_has_valid_parent<T>`]. /// When enabled, runs [`check_hierarchy_component_has_valid_parent<T>`].
/// ///
@ -67,7 +67,7 @@ pub fn check_hierarchy_component_has_valid_parent<T: Component>(
bevy_utils::tracing::warn!( bevy_utils::tracing::warn!(
"warning[B0004]: {name} with the {ty_name} component has a parent without {ty_name}.\n\ "warning[B0004]: {name} with the {ty_name} component has a parent without {ty_name}.\n\
This will cause inconsistent behaviors! See: https://bevyengine.org/learn/errors/b0004", This will cause inconsistent behaviors! See: https://bevyengine.org/learn/errors/b0004",
ty_name = get_short_name(std::any::type_name::<T>()), ty_name = ShortName::of::<T>(),
name = name.map_or_else(|| format!("Entity {}", entity), |s| format!("The {s} entity")), name = name.map_or_else(|| format!("Entity {}", entity), |s| format!("The {s} entity")),
); );
} }

View file

@ -2366,9 +2366,7 @@ bevy_reflect::tests::Test {
fn short_type_path() -> &'static str { fn short_type_path() -> &'static str {
static CELL: GenericTypePathCell = GenericTypePathCell::new(); static CELL: GenericTypePathCell = GenericTypePathCell::new();
CELL.get_or_insert::<Self, _>(|| { CELL.get_or_insert::<Self, _>(|| bevy_utils::ShortName::of::<Self>().to_string())
bevy_utils::get_short_name(std::any::type_name::<Self>())
})
} }
fn type_ident() -> Option<&'static str> { fn type_ident() -> Option<&'static str> {

View file

@ -22,10 +22,8 @@ pub mod prelude {
} }
pub mod futures; pub mod futures;
#[cfg(feature = "alloc")]
mod short_names; mod short_names;
#[cfg(feature = "alloc")] pub use short_names::ShortName;
pub use short_names::get_short_name;
pub mod synccell; pub mod synccell;
pub mod syncunsafecell; pub mod syncunsafecell;

View file

@ -1,62 +1,115 @@
use alloc::string::String; /// Lazily shortens a type name to remove all module paths.
/// Shortens a type name to remove all module paths.
/// ///
/// The short name of a type is its full name as returned by /// 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 /// [`std::any::type_name`], but with the prefix of all paths removed. For
/// example, the short name of `alloc::vec::Vec<core::option::Option<u32>>` /// example, the short name of `alloc::vec::Vec<core::option::Option<u32>>`
/// would be `Vec<Option<u32>>`. /// would be `Vec<Option<u32>>`.
pub fn get_short_name(full_name: &str) -> String { ///
// Generics result in nested paths within <..> blocks. /// Shortening is performed lazily without allocation.
// Consider "bevy_render::camera::camera::extract_cameras<bevy_render::camera::bundle::Camera3d>". #[cfg_attr(
// To tackle this, we parse the string from left to right, collapsing as we go. feature = "alloc",
let mut index: usize = 0; doc = r#" To get a [`String`] from this type, use the [`to_string`](`alloc::string::ToString::to_string`) method."#
let end_of_string = full_name.len(); )]
let mut parsed_name = String::new(); ///
/// # Examples
///
/// ```rust
/// # use bevy_utils::ShortName;
/// #
/// # mod foo {
/// # pub mod bar {
/// # pub struct Baz;
/// # }
/// # }
/// // Baz
/// let short_name = ShortName::of::<foo::bar::Baz>();
/// ```
#[derive(Clone, Copy)]
pub struct ShortName<'a>(pub &'a str);
while index < end_of_string { impl ShortName<'static> {
let rest_of_string = full_name.get(index..end_of_string).unwrap_or_default(); /// Gets a shortened version of the name of the type `T`.
pub fn of<T: ?Sized>() -> Self {
// Collapse everything up to the next special character, Self(core::any::type_name::<T>())
// then skip over it }
if let Some(special_character_index) = rest_of_string.find(|c: char| { }
(c == ' ')
|| (c == '<') impl<'a> ShortName<'a> {
|| (c == '>') /// Gets the original name before shortening.
|| (c == '(') pub const fn original(&self) -> &'a str {
|| (c == ')') self.0
|| (c == '[') }
|| (c == ']') }
|| (c == ',')
|| (c == ';') impl<'a> From<&'a str> for ShortName<'a> {
}) { fn from(value: &'a str) -> Self {
let segment_to_collapse = rest_of_string Self(value)
.get(0..special_character_index) }
.unwrap_or_default(); }
parsed_name += collapse_type_name(segment_to_collapse);
// Insert the special character impl<'a> core::fmt::Debug for ShortName<'a> {
let special_character = fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
&rest_of_string[special_character_index..=special_character_index]; let &ShortName(full_name) = self;
parsed_name.push_str(special_character); // Generics result in nested paths within <..> blocks.
// Consider "bevy_render::camera::camera::extract_cameras<bevy_render::camera::bundle::Camera3d>".
match special_character { // To tackle this, we parse the string from left to right, collapsing as we go.
">" | ")" | "]" let mut index: usize = 0;
if rest_of_string[special_character_index + 1..].starts_with("::") => let end_of_string = full_name.len();
{
parsed_name.push_str("::"); while index < end_of_string {
// Move the index past the "::" let rest_of_string = full_name.get(index..end_of_string).unwrap_or_default();
index += special_character_index + 3;
} // Collapse everything up to the next special character,
// Move the index just past the special character // then skip over it
_ => index += special_character_index + 1, if let Some(special_character_index) = rest_of_string.find(|c: char| {
} (c == ' ')
} else { || (c == '<')
// If there are no special characters left, we're done! || (c == '>')
parsed_name += collapse_type_name(rest_of_string); || (c == '(')
index = end_of_string; || (c == ')')
} || (c == '[')
|| (c == ']')
|| (c == ',')
|| (c == ';')
}) {
let segment_to_collapse = rest_of_string
.get(0..special_character_index)
.unwrap_or_default();
f.write_str(collapse_type_name(segment_to_collapse))?;
// Insert the special character
let special_character =
&rest_of_string[special_character_index..=special_character_index];
f.write_str(special_character)?;
match special_character {
">" | ")" | "]"
if rest_of_string[special_character_index + 1..].starts_with("::") =>
{
f.write_str("::")?;
// Move the index past the "::"
index += special_character_index + 3;
}
// Move the index just past the special character
_ => index += special_character_index + 1,
}
} else {
// If there are no special characters left, we're done!
f.write_str(collapse_type_name(rest_of_string))?;
index = end_of_string;
}
}
Ok(())
}
}
impl<'a> core::fmt::Display for ShortName<'a> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
<Self as core::fmt::Debug>::fmt(self, f)
} }
parsed_name
} }
#[inline(always)] #[inline(always)]
@ -77,49 +130,52 @@ fn collapse_type_name(string: &str) -> &str {
} }
} }
#[cfg(test)] #[cfg(all(test, feature = "alloc"))]
mod name_formatting_tests { mod name_formatting_tests {
use super::get_short_name; use super::ShortName;
#[test] #[test]
fn trivial() { fn trivial() {
assert_eq!(get_short_name("test_system"), "test_system"); assert_eq!(ShortName("test_system").to_string(), "test_system");
} }
#[test] #[test]
fn path_separated() { fn path_separated() {
assert_eq!( assert_eq!(
get_short_name("bevy_prelude::make_fun_game"), ShortName("bevy_prelude::make_fun_game").to_string(),
"make_fun_game" "make_fun_game"
); );
} }
#[test] #[test]
fn tuple_type() { fn tuple_type() {
assert_eq!(get_short_name("(String, String)"), "(String, String)"); assert_eq!(
ShortName("(String, String)").to_string(),
"(String, String)"
);
} }
#[test] #[test]
fn array_type() { fn array_type() {
assert_eq!(get_short_name("[i32; 3]"), "[i32; 3]"); assert_eq!(ShortName("[i32; 3]").to_string(), "[i32; 3]");
} }
#[test] #[test]
fn trivial_generics() { fn trivial_generics() {
assert_eq!(get_short_name("a<B>"), "a<B>"); assert_eq!(ShortName("a<B>").to_string(), "a<B>");
} }
#[test] #[test]
fn multiple_type_parameters() { fn multiple_type_parameters() {
assert_eq!(get_short_name("a<B, C>"), "a<B, C>"); assert_eq!(ShortName("a<B, C>").to_string(), "a<B, C>");
} }
#[test] #[test]
fn enums() { fn enums() {
assert_eq!(get_short_name("Option::None"), "Option::None"); assert_eq!(ShortName("Option::None").to_string(), "Option::None");
assert_eq!(get_short_name("Option::Some(2)"), "Option::Some(2)"); assert_eq!(ShortName("Option::Some(2)").to_string(), "Option::Some(2)");
assert_eq!( assert_eq!(
get_short_name("bevy_render::RenderSet::Prepare"), ShortName("bevy_render::RenderSet::Prepare").to_string(),
"RenderSet::Prepare" "RenderSet::Prepare"
); );
} }
@ -127,7 +183,7 @@ mod name_formatting_tests {
#[test] #[test]
fn generics() { fn generics() {
assert_eq!( assert_eq!(
get_short_name("bevy_render::camera::camera::extract_cameras<bevy_render::camera::bundle::Camera3d>"), ShortName("bevy_render::camera::camera::extract_cameras<bevy_render::camera::bundle::Camera3d>").to_string(),
"extract_cameras<Camera3d>" "extract_cameras<Camera3d>"
); );
} }
@ -135,7 +191,7 @@ mod name_formatting_tests {
#[test] #[test]
fn nested_generics() { fn nested_generics() {
assert_eq!( assert_eq!(
get_short_name("bevy::mad_science::do_mad_science<mad_science::Test<mad_science::Tube>, bavy::TypeSystemAbuse>"), ShortName("bevy::mad_science::do_mad_science<mad_science::Test<mad_science::Tube>, bavy::TypeSystemAbuse>").to_string(),
"do_mad_science<Test<Tube>, TypeSystemAbuse>" "do_mad_science<Test<Tube>, TypeSystemAbuse>"
); );
} }
@ -143,13 +199,16 @@ mod name_formatting_tests {
#[test] #[test]
fn sub_path_after_closing_bracket() { fn sub_path_after_closing_bracket() {
assert_eq!( assert_eq!(
get_short_name("bevy_asset::assets::Assets<bevy_scene::dynamic_scene::DynamicScene>::asset_event_system"), ShortName("bevy_asset::assets::Assets<bevy_scene::dynamic_scene::DynamicScene>::asset_event_system").to_string(),
"Assets<DynamicScene>::asset_event_system" "Assets<DynamicScene>::asset_event_system"
); );
assert_eq!( assert_eq!(
get_short_name("(String, String)::default"), ShortName("(String, String)::default").to_string(),
"(String, String)::default" "(String, String)::default"
); );
assert_eq!(get_short_name("[i32; 16]::default"), "[i32; 16]::default"); assert_eq!(
ShortName("[i32; 16]::default").to_string(),
"[i32; 16]::default"
);
} }
} }