[Merged by Bors] - Cleanups in diagnostics

crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs

@@ -48,6 +48,6 @@ impl<T: Asset> AssetCountDiagnosticsPlugin<T> {
  }
 
  pub fn diagnostic_system(mut diagnostics: ResMut<Diagnostics>, assets: Res<Assets<T>>) {
- diagnostics.add_measurement(Self::diagnostic_id(), assets.len() as f64);
+ diagnostics.add_measurement(Self::diagnostic_id(), || assets.len() as f64);
  }
 }

crates/bevy_diagnostic/src/diagnostic.rs

@@ -37,22 +37,30 @@ pub struct Diagnostic {
  history: VecDeque<DiagnosticMeasurement>,
  sum: f64,
  max_history_length: usize,
+ pub is_enabled: bool,
 }
 
 impl Diagnostic {
+ /// Add a new value as a [`DiagnosticMeasurement`]. Its timestamp will be [`Instant::now`].
  pub fn add_measurement(&mut self, value: f64) {
  let time = Instant::now();
- if self.history.len() == self.max_history_length {
- if let Some(removed_diagnostic) = self.history.pop_front() {
- self.sum -= removed_diagnostic.value;
+ if self.max_history_length > 1 {
+ if self.history.len() == self.max_history_length {
+ if let Some(removed_diagnostic) = self.history.pop_front() {
+ self.sum -= removed_diagnostic.value;
+ }
  }
- }
 
- self.sum += value;
+ self.sum += value;
+ } else {
+ self.history.clear();
+ self.sum = value;
+ }
  self.history
  .push_back(DiagnosticMeasurement { time, value });
  }
 
+ /// Create a new diagnostic with the given ID, name and maximum history.
  pub fn new(
  id: DiagnosticId,
  name: impl Into<Cow<'static, str>>,
@@ -74,28 +82,30 @@ impl Diagnostic {
  history: VecDeque::with_capacity(max_history_length),
  max_history_length,
  sum: 0.0,
+ is_enabled: true,
  }
  }
 
+ /// 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 {
  self.suffix = suffix.into();
  self
  }
 
+ /// Get the latest measurement from this diagnostic.
  #[inline]
  pub fn measurement(&self) -> Option<&DiagnosticMeasurement> {
  self.history.back()
  }
 
+ /// Get the latest value from this diagnostic.
  pub fn value(&self) -> Option<f64> {
  self.measurement().map(|measurement| measurement.value)
  }
 
- pub fn sum(&self) -> f64 {
- self.sum
- }
-
+ /// Return the mean (average) of this diagnostic's values.
+ /// N.B. this a cheap operation as the sum is cached.
  pub fn average(&self) -> Option<f64> {
  if !self.history.is_empty() {
  Some(self.sum / self.history.len() as f64)
@@ -104,10 +114,12 @@ impl Diagnostic {
  }
  }
 
+ /// Return the number of elements for this diagnostic.
  pub fn history_len(&self) -> usize {
  self.history.len()
  }
 
+ /// Return the duration between the oldest and most recent values for this diagnostic.
  pub fn duration(&self) -> Option<Duration> {
  if self.history.len() < 2 {
  return None;
@@ -122,6 +134,7 @@ impl Diagnostic {
  None
  }
 
+ /// Return the maximum number of elements for this diagnostic.
  pub fn get_max_history_length(&self) -> usize {
  self.max_history_length
  }
@@ -133,6 +146,11 @@ impl Diagnostic {
  pub fn measurements(&self) -> impl Iterator<Item = &DiagnosticMeasurement> {
  self.history.iter()
  }
+
+ /// Clear the history of this diagnostic.
+ pub fn clear_history(&mut self) {
+ self.history.clear();
+ }
 }
 
 /// A collection of [Diagnostic]s
@@ -144,6 +162,7 @@ pub struct Diagnostics {
 }
 
 impl Diagnostics {
+ /// Add a new [`Diagnostic`].
  pub fn add(&mut self, diagnostic: Diagnostic) {
  self.diagnostics.insert(diagnostic.id, diagnostic);
  }
@@ -156,18 +175,31 @@ impl Diagnostics {
  self.diagnostics.get_mut(&id)
  }
 
+ /// Get the latest [`DiagnosticMeasurement`] from an enabled [`Diagnostic`].
  pub fn get_measurement(&self, id: DiagnosticId) -> Option<&DiagnosticMeasurement> {
  self.diagnostics
  .get(&id)
+ .filter(|diagnostic| diagnostic.is_enabled)
  .and_then(|diagnostic| diagnostic.measurement())
  }
 
- pub fn add_measurement(&mut self, id: DiagnosticId, value: f64) {
- if let Some(diagnostic) = self.diagnostics.get_mut(&id) {
- diagnostic.add_measurement(value);
+ /// 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)
+ where
+ F: FnOnce() -> f64,
+ {
+ if let Some(diagnostic) = self
+ .diagnostics
+ .get_mut(&id)
+ .filter(|diagnostic| diagnostic.is_enabled)
+ {
+ diagnostic.add_measurement(value());
  }
  }
 
+ /// Return an iterator over all [`Diagnostic`].
  pub fn iter(&self) -> impl Iterator<Item = &Diagnostic> {
  self.diagnostics.values()
  }

crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs

@@ -1,8 +1,5 @@
 use bevy_app::{App, Plugin};
-use bevy_ecs::{
- system::{IntoExclusiveSystem, ResMut},
- world::World,
-};
+use bevy_ecs::{entity::Entities, system::ResMut};
 
 use crate::{Diagnostic, DiagnosticId, Diagnostics};
 
@@ -13,7 +10,7 @@ pub struct EntityCountDiagnosticsPlugin;
 impl Plugin for EntityCountDiagnosticsPlugin {
  fn build(&self, app: &mut App) {
  app.add_startup_system(Self::setup_system)
- .add_system(Self::diagnostic_system.exclusive_system());
+ .add_system(Self::diagnostic_system);
  }
 }
 
@@ -25,10 +22,7 @@ impl EntityCountDiagnosticsPlugin {
  diagnostics.add(Diagnostic::new(Self::ENTITY_COUNT, "entity_count", 20));
  }
 
- pub fn diagnostic_system(world: &mut World) {
- let entity_count = world.entities().len();
- if let Some(mut diagnostics) = world.get_resource_mut::<Diagnostics>() {
- diagnostics.add_measurement(Self::ENTITY_COUNT, entity_count as f64);
- }
+ pub fn diagnostic_system(mut diagnostics: ResMut<Diagnostics>, entities: &Entities) {
+ diagnostics.add_measurement(Self::ENTITY_COUNT, || entities.len() as f64);
  }
 }

crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs

@@ -37,29 +37,23 @@ impl FrameTimeDiagnosticsPlugin {
  time: Res<Time>,
  mut state: ResMut<FrameTimeDiagnosticsState>,
  ) {
- state.frame_count = state.frame_count.wrapping_add(1);
- diagnostics.add_measurement(Self::FRAME_COUNT, state.frame_count as f64);
+ diagnostics.add_measurement(Self::FRAME_COUNT, || {
+ state.frame_count = state.frame_count.wrapping_add(1);
+ state.frame_count as f64
+ });
 
  if time.delta_seconds_f64() == 0.0 {
  return;
  }
 
- diagnostics.add_measurement(Self::FRAME_TIME, time.delta_seconds_f64());
- if let Some(fps) = diagnostics
- .get(Self::FRAME_TIME)
- .and_then(|frame_time_diagnostic| {
- frame_time_diagnostic
- .average()
- .and_then(|frame_time_average| {
- if frame_time_average > 0.0 {
- Some(1.0 / frame_time_average)
- } else {
- None
- }
- })
- })
- {
- diagnostics.add_measurement(Self::FPS, fps);
- }
+ diagnostics.add_measurement(Self::FRAME_TIME, || time.delta_seconds_f64());
+
+ diagnostics.add_measurement(Self::FPS, || 1.0 / time.delta_seconds_f64());
+ }
+}
+
+impl FrameTimeDiagnosticsState {
+ pub fn reset_frame_count(&mut self) {
+ self.frame_count = 0;
  }
 }

crates/bevy_diagnostic/src/log_diagnostics_plugin.rs

@@ -53,30 +53,29 @@ impl LogDiagnosticsPlugin {
 
  fn log_diagnostic(diagnostic: &Diagnostic) {
  if let Some(value) = diagnostic.value() {
- if let Some(average) = diagnostic.average() {
- info!(
- target: "bevy diagnostic",
- // Suffix is only used for 's' as in seconds currently,
- // so we reserve one column for it; however,
- // Do not reserve one column for the suffix in the average
- // The ) hugging the value is more aesthetically pleasing
- "{name:<name_width$}: {value:>11.6}{suffix:1} (avg {average:>.6}{suffix:})",
- name = diagnostic.name,
- value = value,
- suffix = diagnostic.suffix,
- average = average,
- name_width = crate::MAX_DIAGNOSTIC_NAME_WIDTH,
- );
- } else {
- info!(
- target: "bevy diagnostic",
- "{name:<name_width$}: {value:>.6}{suffix:}",
- name = diagnostic.name,
- value = value,
- suffix = diagnostic.suffix,
- name_width = crate::MAX_DIAGNOSTIC_NAME_WIDTH,
- );
+ if diagnostic.get_max_history_length() > 1 {
+ if let Some(average) = diagnostic.average() {
+ info!(
+ target: "bevy diagnostic",
+ // Suffix is only used for 's' as in seconds currently,
+ // so we reserve one column for it; however,
+ // Do not reserve one column for the suffix in the average
+ // The ) hugging the value is more aesthetically pleasing
+ "{name:<name_width$}: {value:>11.6}{suffix:1} (avg {average:>.6}{suffix:})",
+ name = diagnostic.name,
+ suffix = diagnostic.suffix,
+ name_width = crate::MAX_DIAGNOSTIC_NAME_WIDTH,
+ );
+ return;
+ }
  }
+ info!(
+ target: "bevy diagnostic",
+ "{name:<name_width$}: {value:>.6}{suffix:}",
+ name = diagnostic.name,
+ suffix = diagnostic.suffix,
+ name_width = crate::MAX_DIAGNOSTIC_NAME_WIDTH,
+ );
  }
  }
 
@@ -87,11 +86,18 @@ impl LogDiagnosticsPlugin {
  ) {
  if state.timer.tick(time.delta()).finished() {
  if let Some(ref filter) = state.filter {
- for diagnostic in filter.iter().map(|id| diagnostics.get(*id).unwrap()) {
+ for diagnostic in filter.iter().flat_map(|id| {
+ diagnostics
+ .get(*id)
+ .filter(|diagnostic| diagnostic.is_enabled)
+ }) {
  Self::log_diagnostic(diagnostic);
  }
  } else {
- for diagnostic in diagnostics.iter() {
+ for diagnostic in diagnostics
+ .iter()
+ .filter(|diagnostic| diagnostic.is_enabled)
+ {
  Self::log_diagnostic(diagnostic);
  }
  }
@@ -105,11 +111,18 @@ impl LogDiagnosticsPlugin {
  ) {
  if state.timer.tick(time.delta()).finished() {
  if let Some(ref filter) = state.filter {
- for diagnostic in filter.iter().map(|id| diagnostics.get(*id).unwrap()) {
+ for diagnostic in filter.iter().flat_map(|id| {
+ diagnostics
+ .get(*id)
+ .filter(|diagnostic| diagnostic.is_enabled)
+ }) {
  debug!("{:#?}\n", diagnostic);
  }
  } else {
- for diagnostic in diagnostics.iter() {
+ for diagnostic in diagnostics
+ .iter()
+ .filter(|diagnostic| diagnostic.is_enabled)
+ {
  debug!("{:#?}\n", diagnostic);
  }
  }

examples/diagnostics/custom_diagnostic.rs

@@ -33,5 +33,5 @@ fn setup_diagnostic_system(mut diagnostics: ResMut<Diagnostics>) {
 
 fn my_system(mut diagnostics: ResMut<Diagnostics>) {
  // Add a measurement of 10.0 for our diagnostic each time this system runs.
- diagnostics.add_measurement(SYSTEM_ITERATION_COUNT, 10.0);
+ diagnostics.add_measurement(SYSTEM_ITERATION_COUNT, || 10.0);
 }