Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace DiagnosticId by DiagnosticPath #9266

Merged
merged 5 commits into from
Jan 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/bevy_diagnostic/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
186 changes: 133 additions & 53 deletions crates/bevy_diagnostic/src/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can we swap "components" here to "parts" to be less confusing in a Bevy context?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest from_sections

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

Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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> {
Expand Down Expand Up @@ -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())
}
Expand All @@ -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(
Expand All @@ -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);
}
}
Expand All @@ -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();
/// ```
Expand Down
9 changes: 4 additions & 5 deletions crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs
Original file line number Diff line number Diff line change
@@ -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.
///
Expand All @@ -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);
}
}
30 changes: 12 additions & 18 deletions crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs
Original file line number Diff line number Diff line change
@@ -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::*;
Expand All @@ -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);
}
}
5 changes: 2 additions & 3 deletions crates/bevy_diagnostic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Loading