Replace DiagnosticId by DiagnosticPath (#9266)

# Objective

Implements #9216 

## Solution

- Replace `DiagnosticId` by `DiagnosticPath`. It's pre-hashed using
`const-fnv1a-hash` crate, so it's possible to create path in const
contexts.

---

## Changelog

- Replaced `DiagnosticId` by `DiagnosticPath`
- Set default history length to 120 measurements (2 seconds on 60 fps).

I've noticed hardcoded constant 20 everywhere and decided to change it
to `DEFAULT_MAX_HISTORY_LENGTH` , which is set to new diagnostics by
default. To override it, use `with_max_history_length`.


## Migration Guide

```diff
- const UNIQUE_DIAG_ID: DiagnosticId = DiagnosticId::from_u128(42);
+ const UNIQUE_DIAG_PATH: DiagnosticPath = DiagnosticPath::const_new("foo/bar");

- Diagnostic::new(UNIQUE_DIAG_ID, "example", 10)
+ Diagnostic::new(UNIQUE_DIAG_PATH).with_max_history_length(10)

- diagnostics.add_measurement(UNIQUE_DIAG_ID, || 42);
+ diagnostics.add_measurement(&UNIQUE_DIAG_ID, || 42);
```

Author: LeshaInc

crates/bevy_diagnostic/Cargo.toml

@@ -21,6 +21,8 @@ bevy_log = { path = "../bevy_log", version = "0.12.0" }
 bevy_time = { path = "../bevy_time", version = "0.12.0" }
 bevy_utils = { path = "../bevy_utils", version = "0.12.0" }
 
+const-fnv1a-hash = "1.1.0"
+
 # MacOS
 [target.'cfg(all(target_os="macos"))'.dependencies]
 # Some features of sysinfo are not supported by apple. This will disable those features on apple devices

crates/bevy_diagnostic/src/diagnostic.rs

@@ -1,24 +1,108 @@
+use std::hash::{Hash, Hasher};
+use std::{borrow::Cow, collections::VecDeque};
+
 use bevy_app::App;
 use bevy_ecs::system::{Deferred, Res, Resource, SystemBuffer, SystemParam};
-use bevy_log::warn;
-use bevy_utils::{Duration, Instant, StableHashMap, Uuid};
-use std::{borrow::Cow, collections::VecDeque};
+use bevy_utils::{hashbrown::HashMap, Duration, Instant, PassHash};
+use const_fnv1a_hash::fnv1a_hash_str_64;
+
+use crate::DEFAULT_MAX_HISTORY_LENGTH;
+
+/// Unique diagnostic path, separated by `/`.
+///
+/// Requirements:
+/// - Can't be empty
+/// - Can't have leading or trailing `/`
+/// - Can't have empty components.
+#[derive(Debug, Clone)]
+pub struct DiagnosticPath {
+    path: Cow<'static, str>,
+    hash: u64,
+}
+
+impl DiagnosticPath {
+    /// Create a new `DiagnosticPath`. Usable in const contexts.
+    ///
+    /// **Note**: path is not validated, so make sure it follows all the requirements.
+    pub const fn const_new(path: &'static str) -> DiagnosticPath {
+        DiagnosticPath {
+            path: Cow::Borrowed(path),
+            hash: fnv1a_hash_str_64(path),
+        }
+    }
+
+    /// Create a new `DiagnosticPath` from the specified string.
+    pub fn new(path: impl Into<Cow<'static, str>>) -> DiagnosticPath {
+        let path = path.into();
+
+        debug_assert!(!path.is_empty(), "diagnostic path can't be empty");
+        debug_assert!(
+            !path.starts_with('/'),
+            "diagnostic path can't be start with `/`"
+        );
+        debug_assert!(
+            !path.ends_with('/'),
+            "diagnostic path can't be end with `/`"
+        );
+        debug_assert!(
+            !path.contains("//"),
+            "diagnostic path can't contain empty components"
+        );
+
+        DiagnosticPath {
+            hash: fnv1a_hash_str_64(&path),
+            path,
+        }
+    }
+
+    /// Create a new `DiagnosticPath` from an iterator over components.
+    pub fn from_components<'a>(components: impl IntoIterator<Item = &'a str>) -> DiagnosticPath {
+        let mut buf = String::new();
+
+        for (i, component) in components.into_iter().enumerate() {
+            if i > 0 {
+                buf.push('/');
+            }
+            buf.push_str(component);
+        }
+
+        DiagnosticPath::new(buf)
+    }
 
-use crate::MAX_DIAGNOSTIC_NAME_WIDTH;
+    /// Returns full path, joined by `/`
+    pub fn as_str(&self) -> &str {
+        &self.path
+    }
 
-/// Unique identifier for a [`Diagnostic`].
-#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq, PartialOrd, Ord)]
-pub struct DiagnosticId(pub Uuid);
+    /// Returns an iterator over path components.
+    pub fn components(&self) -> impl Iterator<Item = &str> + '_ {
+        self.path.split('/')
+    }
+}
 
-impl DiagnosticId {
-    pub const fn from_u128(value: u128) -> Self {
-        DiagnosticId(Uuid::from_u128(value))
+impl From<DiagnosticPath> for String {
+    fn from(path: DiagnosticPath) -> Self {
+        path.path.into()
     }
 }
 
-impl Default for DiagnosticId {
-    fn default() -> Self {
-        DiagnosticId(Uuid::new_v4())
+impl Eq for DiagnosticPath {}
+
+impl PartialEq for DiagnosticPath {
+    fn eq(&self, other: &Self) -> bool {
+        self.hash == other.hash && self.path == other.path
+    }
+}
+
+impl Hash for DiagnosticPath {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        state.write_u64(self.hash);
+    }
+}
+
+impl std::fmt::Display for DiagnosticPath {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.path.fmt(f)
     }
 }
 
@@ -33,8 +117,7 @@ pub struct DiagnosticMeasurement {
 /// Diagnostic examples: frames per second, CPU usage, network latency
 #[derive(Debug)]
 pub struct Diagnostic {
-    pub id: DiagnosticId,
-    pub name: Cow<'static, str>,
+    path: DiagnosticPath,
     pub suffix: Cow<'static, str>,
     history: VecDeque<DiagnosticMeasurement>,
     sum: f64,
@@ -71,34 +154,29 @@ impl Diagnostic {
         self.history.push_back(measurement);
     }
 
-    /// Create a new diagnostic with the given ID, name and maximum history.
-    pub fn new(
-        id: DiagnosticId,
-        name: impl Into<Cow<'static, str>>,
-        max_history_length: usize,
-    ) -> Diagnostic {
-        let name = name.into();
-        if name.chars().count() > MAX_DIAGNOSTIC_NAME_WIDTH {
-            // This could be a false positive due to a unicode width being shorter
-            warn!(
-                "Diagnostic {:?} has name longer than {} characters, and so might overflow in the LogDiagnosticsPlugin\
-                Consider using a shorter name.",
-                name, MAX_DIAGNOSTIC_NAME_WIDTH
-            );
-        }
+    /// Create a new diagnostic with the given path.
+    pub fn new(path: DiagnosticPath) -> Diagnostic {
         Diagnostic {
-            id,
-            name,
+            path,
             suffix: Cow::Borrowed(""),
-            history: VecDeque::with_capacity(max_history_length),
-            max_history_length,
+            history: VecDeque::with_capacity(DEFAULT_MAX_HISTORY_LENGTH),
+            max_history_length: DEFAULT_MAX_HISTORY_LENGTH,
             sum: 0.0,
             ema: 0.0,
             ema_smoothing_factor: 2.0 / 21.0,
             is_enabled: true,
         }
     }
 
+    /// Set the maximum history length.
+    #[must_use]
+    pub fn with_max_history_length(mut self, max_history_length: usize) -> Self {
+        self.max_history_length = max_history_length;
+        self.history.reserve(self.max_history_length);
+        self.history.shrink_to(self.max_history_length);
+        self
+    }
+
     /// Add a suffix to use when logging the value, can be used to show a unit.
     #[must_use]
     pub fn with_suffix(mut self, suffix: impl Into<Cow<'static, str>>) -> Self {
@@ -122,6 +200,10 @@ impl Diagnostic {
         self
     }
 
+    pub fn path(&self) -> &DiagnosticPath {
+        &self.path
+    }
+
     /// Get the latest measurement from this diagnostic.
     #[inline]
     pub fn measurement(&self) -> Option<&DiagnosticMeasurement> {
@@ -198,31 +280,29 @@ impl Diagnostic {
 /// A collection of [`Diagnostic`]s.
 #[derive(Debug, Default, Resource)]
 pub struct DiagnosticsStore {
-    // This uses a [`StableHashMap`] to ensure that the iteration order is deterministic between
-    // runs when all diagnostics are inserted in the same order.
-    diagnostics: StableHashMap<DiagnosticId, Diagnostic>,
+    diagnostics: HashMap<DiagnosticPath, Diagnostic, PassHash>,
 }
 
 impl DiagnosticsStore {
     /// Add a new [`Diagnostic`].
     ///
     /// If possible, prefer calling [`App::register_diagnostic`].
     pub fn add(&mut self, diagnostic: Diagnostic) {
-        self.diagnostics.insert(diagnostic.id, diagnostic);
+        self.diagnostics.insert(diagnostic.path.clone(), diagnostic);
     }
 
-    pub fn get(&self, id: DiagnosticId) -> Option<&Diagnostic> {
-        self.diagnostics.get(&id)
+    pub fn get(&self, path: &DiagnosticPath) -> Option<&Diagnostic> {
+        self.diagnostics.get(path)
     }
 
-    pub fn get_mut(&mut self, id: DiagnosticId) -> Option<&mut Diagnostic> {
-        self.diagnostics.get_mut(&id)
+    pub fn get_mut(&mut self, path: &DiagnosticPath) -> Option<&mut Diagnostic> {
+        self.diagnostics.get_mut(path)
     }
 
     /// Get the latest [`DiagnosticMeasurement`] from an enabled [`Diagnostic`].
-    pub fn get_measurement(&self, id: DiagnosticId) -> Option<&DiagnosticMeasurement> {
+    pub fn get_measurement(&self, path: &DiagnosticPath) -> Option<&DiagnosticMeasurement> {
         self.diagnostics
-            .get(&id)
+            .get(path)
             .filter(|diagnostic| diagnostic.is_enabled)
             .and_then(|diagnostic| diagnostic.measurement())
     }
@@ -249,27 +329,27 @@ impl<'w, 's> Diagnostics<'w, 's> {
     /// Add a measurement to an enabled [`Diagnostic`]. The measurement is passed as a function so that
     /// it will be evaluated only if the [`Diagnostic`] is enabled. This can be useful if the value is
     /// costly to calculate.
-    pub fn add_measurement<F>(&mut self, id: DiagnosticId, value: F)
+    pub fn add_measurement<F>(&mut self, path: &DiagnosticPath, value: F)
     where
         F: FnOnce() -> f64,
     {
         if self
             .store
-            .get(id)
+            .get(path)
             .filter(|diagnostic| diagnostic.is_enabled)
             .is_some()
         {
             let measurement = DiagnosticMeasurement {
                 time: Instant::now(),
                 value: value(),
             };
-            self.queue.0.insert(id, measurement);
+            self.queue.0.insert(path.clone(), measurement);
         }
     }
 }
 
 #[derive(Default)]
-struct DiagnosticsBuffer(StableHashMap<DiagnosticId, DiagnosticMeasurement>);
+struct DiagnosticsBuffer(HashMap<DiagnosticPath, DiagnosticMeasurement, PassHash>);
 
 impl SystemBuffer for DiagnosticsBuffer {
     fn apply(
@@ -278,8 +358,8 @@ impl SystemBuffer for DiagnosticsBuffer {
         world: &mut bevy_ecs::world::World,
     ) {
         let mut diagnostics = world.resource_mut::<DiagnosticsStore>();
-        for (id, measurement) in self.0.drain() {
-            if let Some(diagnostic) = diagnostics.get_mut(id) {
+        for (path, measurement) in self.0.drain() {
+            if let Some(diagnostic) = diagnostics.get_mut(&path) {
                 diagnostic.add_measurement(measurement);
             }
         }
@@ -298,12 +378,12 @@ impl RegisterDiagnostic for App {
     ///
     /// ```
     /// use bevy_app::App;
-    /// use bevy_diagnostic::{Diagnostic, DiagnosticsPlugin, DiagnosticId, RegisterDiagnostic};
+    /// use bevy_diagnostic::{Diagnostic, DiagnosticsPlugin, DiagnosticPath, RegisterDiagnostic};
     ///
-    /// const UNIQUE_DIAG_ID: DiagnosticId = DiagnosticId::from_u128(42);
+    /// const UNIQUE_DIAG_PATH: DiagnosticPath = DiagnosticPath::const_new("foo/bar");
     ///
     /// App::new()
-    ///     .register_diagnostic(Diagnostic::new(UNIQUE_DIAG_ID, "example", 10))
+    ///     .register_diagnostic(Diagnostic::new(UNIQUE_DIAG_PATH))
     ///     .add_plugins(DiagnosticsPlugin)
     ///     .run();
     /// ```

crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs

@@ -1,7 +1,7 @@
 use bevy_app::prelude::*;
 use bevy_ecs::entity::Entities;
 
-use crate::{Diagnostic, DiagnosticId, Diagnostics, RegisterDiagnostic};
+use crate::{Diagnostic, DiagnosticPath, Diagnostics, RegisterDiagnostic};
 
 /// Adds "entity count" diagnostic to an App.
 ///
@@ -13,16 +13,15 @@ pub struct EntityCountDiagnosticsPlugin;
 
 impl Plugin for EntityCountDiagnosticsPlugin {
     fn build(&self, app: &mut App) {
-        app.register_diagnostic(Diagnostic::new(Self::ENTITY_COUNT, "entity_count", 20))
+        app.register_diagnostic(Diagnostic::new(Self::ENTITY_COUNT))
             .add_systems(Update, Self::diagnostic_system);
     }
 }
 
 impl EntityCountDiagnosticsPlugin {
-    pub const ENTITY_COUNT: DiagnosticId =
-        DiagnosticId::from_u128(187513512115068938494459732780662867798);
+    pub const ENTITY_COUNT: DiagnosticPath = DiagnosticPath::const_new("entity_count");
 
     pub fn diagnostic_system(mut diagnostics: Diagnostics, entities: &Entities) {
-        diagnostics.add_measurement(Self::ENTITY_COUNT, || entities.len() as f64);
+        diagnostics.add_measurement(&Self::ENTITY_COUNT, || entities.len() as f64);
     }
 }

crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs

@@ -1,4 +1,4 @@
-use crate::{Diagnostic, DiagnosticId, Diagnostics, RegisterDiagnostic};
+use crate::{Diagnostic, DiagnosticPath, Diagnostics, RegisterDiagnostic};
 use bevy_app::prelude::*;
 use bevy_core::FrameCount;
 use bevy_ecs::prelude::*;
@@ -13,39 +13,33 @@ use bevy_time::{Real, Time};
 pub struct FrameTimeDiagnosticsPlugin;
 
 impl Plugin for FrameTimeDiagnosticsPlugin {
-    fn build(&self, app: &mut App) {
-        app.register_diagnostic(
-            Diagnostic::new(Self::FRAME_TIME, "frame_time", 20).with_suffix("ms"),
-        )
-        .register_diagnostic(Diagnostic::new(Self::FPS, "fps", 20))
-        .register_diagnostic(
-            Diagnostic::new(Self::FRAME_COUNT, "frame_count", 1).with_smoothing_factor(0.0),
-        )
-        .add_systems(Update, Self::diagnostic_system);
+    fn build(&self, app: &mut bevy_app::App) {
+        app.register_diagnostic(Diagnostic::new(Self::FRAME_TIME).with_suffix("ms"))
+            .register_diagnostic(Diagnostic::new(Self::FPS))
+            .register_diagnostic(Diagnostic::new(Self::FRAME_COUNT).with_smoothing_factor(0.0))
+            .add_systems(Update, Self::diagnostic_system);
     }
 }
 
 impl FrameTimeDiagnosticsPlugin {
-    pub const FPS: DiagnosticId = DiagnosticId::from_u128(288146834822086093791974408528866909483);
-    pub const FRAME_COUNT: DiagnosticId =
-        DiagnosticId::from_u128(54021991829115352065418785002088010277);
-    pub const FRAME_TIME: DiagnosticId =
-        DiagnosticId::from_u128(73441630925388532774622109383099159699);
+    pub const FPS: DiagnosticPath = DiagnosticPath::const_new("fps");
+    pub const FRAME_COUNT: DiagnosticPath = DiagnosticPath::const_new("frame_count");
+    pub const FRAME_TIME: DiagnosticPath = DiagnosticPath::const_new("frame_time");
 
     pub fn diagnostic_system(
         mut diagnostics: Diagnostics,
         time: Res<Time<Real>>,
         frame_count: Res<FrameCount>,
     ) {
-        diagnostics.add_measurement(Self::FRAME_COUNT, || frame_count.0 as f64);
+        diagnostics.add_measurement(&Self::FRAME_COUNT, || frame_count.0 as f64);
 
         let delta_seconds = time.delta_seconds_f64();
         if delta_seconds == 0.0 {
             return;
         }
 
-        diagnostics.add_measurement(Self::FRAME_TIME, || delta_seconds * 1000.0);
+        diagnostics.add_measurement(&Self::FRAME_TIME, || delta_seconds * 1000.0);
 
-        diagnostics.add_measurement(Self::FPS, || 1.0 / delta_seconds);
+        diagnostics.add_measurement(&Self::FPS, || 1.0 / delta_seconds);
     }
 }

crates/bevy_diagnostic/src/lib.rs

@@ -28,6 +28,5 @@ impl Plugin for DiagnosticsPlugin {
     }
 }
 
-/// The width which diagnostic names will be printed as
-/// Plugin names should not be longer than this value
-pub const MAX_DIAGNOSTIC_NAME_WIDTH: usize = 32;
+/// Default max history length for new diagnostics.
+pub const DEFAULT_MAX_HISTORY_LENGTH: usize = 120;