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

updates on diagnostics (log + new diagnostics) #1085

Merged
merged 7 commits into from
Dec 24, 2020
Merged
Show file tree
Hide file tree
Changes from 5 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
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ name = "custom_diagnostic"
path = "examples/diagnostics/custom_diagnostic.rs"

[[example]]
name = "print_diagnostics"
path = "examples/diagnostics/print_diagnostics.rs"
name = "log_diagnostics"
path = "examples/diagnostics/log_diagnostics.rs"

[[example]]
name = "event"
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_asset/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ filesystem_watcher = ["notify"]
[dependencies]
# bevy
bevy_app = { path = "../bevy_app", version = "0.4.0" }
bevy_diagnostic = { path = "../bevy_diagnostic", version = "0.4.0" }
bevy_ecs = { path = "../bevy_ecs", version = "0.4.0" }
bevy_reflect = { path = "../bevy_reflect", version = "0.4.0", features = ["bevy"] }
bevy_tasks = { path = "../bevy_tasks", version = "0.4.0" }
Expand Down
35 changes: 35 additions & 0 deletions crates/bevy_asset/src/diagnostic/asset_count_diagnostics_plugin.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use crate::{Asset, Assets};
use bevy_app::prelude::*;
use bevy_diagnostic::{Diagnostic, DiagnosticId, Diagnostics};
use bevy_ecs::{IntoSystem, Res, ResMut};

/// Adds "asset count" diagnostic to an App
#[derive(Default)]
pub struct AssetCountDiagnosticsPlugin<T: Asset> {
marker: std::marker::PhantomData<T>,
}

impl<T: Asset> Plugin for AssetCountDiagnosticsPlugin<T> {
fn build(&self, app: &mut AppBuilder) {
app.add_startup_system(Self::setup_system.system())
.add_system(Self::diagnostic_system.system());
}
}

impl<T: Asset> AssetCountDiagnosticsPlugin<T> {
pub fn diagnostic_id() -> DiagnosticId {
DiagnosticId(T::TYPE_UUID)
}

pub fn setup_system(mut diagnostics: ResMut<Diagnostics>) {
diagnostics.add(Diagnostic::new(
Self::diagnostic_id(),
&format!("asset_count {}", std::any::type_name::<T>()),
20,
));
}

pub fn diagnostic_system(mut diagnostics: ResMut<Diagnostics>, assets: Res<Assets<T>>) {
diagnostics.add_measurement(Self::diagnostic_id(), assets.len() as f64);
}
}
2 changes: 2 additions & 0 deletions crates/bevy_asset/src/diagnostic/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
mod asset_count_diagnostics_plugin;
pub use asset_count_diagnostics_plugin::AssetCountDiagnosticsPlugin;
1 change: 1 addition & 0 deletions crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod asset_server;
mod assets;
pub mod diagnostic;
#[cfg(all(
feature = "filesystem_watcher",
all(not(target_arch = "wasm32"), not(target_os = "android"))
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_diagnostic/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ keywords = ["bevy"]
bevy_app = { path = "../bevy_app", version = "0.4.0" }
bevy_core = { path = "../bevy_core", version = "0.4.0" }
bevy_ecs = { path = "../bevy_ecs", version = "0.4.0" }
bevy_log = { path = "../bevy_log", version = "0.4.0" }
bevy_utils = { path = "../bevy_utils", version = "0.4.0" }

# other
Expand Down
8 changes: 4 additions & 4 deletions crates/bevy_diagnostic/src/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use bevy_utils::{Duration, HashMap, Instant, Uuid};
use std::collections::VecDeque;
use bevy_utils::{Duration, Instant, Uuid};
use std::collections::{BTreeMap, VecDeque};

/// Unique identifier for a [Diagnostic]
#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)]
#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq, PartialOrd, Ord)]
pub struct DiagnosticId(pub Uuid);

impl DiagnosticId {
Expand Down Expand Up @@ -101,7 +101,7 @@ impl Diagnostic {
/// A collection of [Diagnostic]s
#[derive(Debug, Default)]
pub struct Diagnostics {
diagnostics: HashMap<DiagnosticId, Diagnostic>,
diagnostics: BTreeMap<DiagnosticId, Diagnostic>,
}

impl Diagnostics {
Expand Down
31 changes: 31 additions & 0 deletions crates/bevy_diagnostic/src/entity_count_diagnostics_plugin.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
use crate::{Diagnostic, DiagnosticId, Diagnostics};
use bevy_app::prelude::*;
use bevy_ecs::{Entity, IntoSystem, Query, ResMut};

/// Adds "entity count" diagnostic to an App
#[derive(Default)]
pub struct EntityCountDiagnosticsPlugin;

impl Plugin for EntityCountDiagnosticsPlugin {
fn build(&self, app: &mut AppBuilder) {
app.add_startup_system(Self::setup_system.system())
.add_system(Self::diagnostic_system.system());
}
}

impl EntityCountDiagnosticsPlugin {
pub const ENTITY_COUNT: DiagnosticId =
DiagnosticId::from_u128(187513512115068938494459732780662867798);

pub fn setup_system(mut diagnostics: ResMut<Diagnostics>) {
diagnostics.add(Diagnostic::new(Self::ENTITY_COUNT, "entity_count", 20));
}

pub fn diagnostic_system(mut diagnostics: ResMut<Diagnostics>, query: Query<Entity>) {
let mut count = 0.;
for _entity in &mut query.iter() {
Copy link
Member

Choose a reason for hiding this comment

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

we could replace this with let count = query.iter().count(). Thats simpler, but also if/when we re-implement ExactSizeIterator for query it could make this O(1)

Copy link
Member

Choose a reason for hiding this comment

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

Or alternatively we could make it O(1) now by adding a world.entity_count() function and making this system thread-local.

Your call!

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried adding it to World, in my test with a system adding and removing entities every frame, the two counts are the same

count += 1.;
}
diagnostics.add_measurement(Self::ENTITY_COUNT, count);
}
}
6 changes: 4 additions & 2 deletions crates/bevy_diagnostic/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
mod diagnostic;
mod entity_count_diagnostics_plugin;
mod frame_time_diagnostics_plugin;
mod print_diagnostics_plugin;
mod log_diagnostics_plugin;
pub use diagnostic::*;
pub use entity_count_diagnostics_plugin::EntityCountDiagnosticsPlugin;
pub use frame_time_diagnostics_plugin::FrameTimeDiagnosticsPlugin;
pub use print_diagnostics_plugin::PrintDiagnosticsPlugin;
pub use log_diagnostics_plugin::LogDiagnosticsPlugin;

use bevy_app::prelude::*;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,103 +2,102 @@ use super::{Diagnostic, DiagnosticId, Diagnostics};
use bevy_app::prelude::*;
use bevy_core::{Time, Timer};
use bevy_ecs::{IntoSystem, Res, ResMut};
use bevy_log::{debug, info};
use bevy_utils::Duration;

/// An App Plugin that prints diagnostics to the console
pub struct PrintDiagnosticsPlugin {
/// An App Plugin that logs diagnostics to the console
pub struct LogDiagnosticsPlugin {
pub debug: bool,
pub wait_duration: Duration,
pub filter: Option<Vec<DiagnosticId>>,
}

/// State used by the [PrintDiagnosticsPlugin]
pub struct PrintDiagnosticsState {
/// State used by the [LogDiagnosticsPlugin]
struct LogDiagnosticsState {
timer: Timer,
filter: Option<Vec<DiagnosticId>>,
}

impl Default for PrintDiagnosticsPlugin {
impl Default for LogDiagnosticsPlugin {
fn default() -> Self {
PrintDiagnosticsPlugin {
LogDiagnosticsPlugin {
debug: false,
wait_duration: Duration::from_secs(1),
filter: None,
}
}
}

impl Plugin for PrintDiagnosticsPlugin {
impl Plugin for LogDiagnosticsPlugin {
fn build(&self, app: &mut bevy_app::AppBuilder) {
app.add_resource(PrintDiagnosticsState {
app.add_resource(LogDiagnosticsState {
timer: Timer::new(self.wait_duration, true),
filter: self.filter.clone(),
});

if self.debug {
app.add_system_to_stage(
stage::POST_UPDATE,
Self::print_diagnostics_debug_system.system(),
Self::log_diagnostics_debug_system.system(),
);
} else {
app.add_system_to_stage(stage::POST_UPDATE, Self::print_diagnostics_system.system());
app.add_system_to_stage(stage::POST_UPDATE, Self::log_diagnostics_system.system());
}
}
}

impl PrintDiagnosticsPlugin {
impl LogDiagnosticsPlugin {
pub fn filtered(filter: Vec<DiagnosticId>) -> Self {
PrintDiagnosticsPlugin {
LogDiagnosticsPlugin {
filter: Some(filter),
..Default::default()
}
}

fn print_diagnostic(diagnostic: &Diagnostic) {
fn log_diagnostic(diagnostic: &Diagnostic) {
if let Some(value) = diagnostic.value() {
print!("{:<65}: {:<10.6}", diagnostic.name, value);
if let Some(average) = diagnostic.average() {
print!(" (avg {:.6})", average);
info!(
"{:<65}: {:<10.6} (avg {:.6})",
diagnostic.name, value, average
);
} else {
info!("{:<65}: {:<10.6}", diagnostic.name, value);
}

println!("\n");
}
}

pub fn print_diagnostics_system(
mut state: ResMut<PrintDiagnosticsState>,
fn log_diagnostics_system(
mut state: ResMut<LogDiagnosticsState>,
time: Res<Time>,
diagnostics: Res<Diagnostics>,
) {
if state.timer.tick(time.delta_seconds()).finished() {
println!("Diagnostics:");
println!("{}", "-".repeat(93));
if let Some(ref filter) = state.filter {
for diagnostic in filter.iter().map(|id| diagnostics.get(*id).unwrap()) {
Self::print_diagnostic(diagnostic);
Self::log_diagnostic(diagnostic);
}
} else {
for diagnostic in diagnostics.iter() {
Copy link
Member

Choose a reason for hiding this comment

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

Can these be sorted so that the diagnostics are in a consistent order?

It's not helpful every time you restart the app getting a different order.

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently you can have them ordered if you use a filter, as the filter is stored in a Vec and then iterated over.

Without filter, the diagnostics are stored in a HashMap and that doesn't keep order... We could switch to a BTreeMap, or add a Vec to keep order of insertion and use it to iterate... What do you think?

Copy link
Member

@DJMcNab DJMcNab Dec 22, 2020

Choose a reason for hiding this comment

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

So BTreeMap would probably be a good start, although a Vec sorted by name or by insertion order could both also be a good choice.

Basically, anything deterministic would be better than the current situation.

Edit: Given that there are generally only a handful of diagnostic types, it might make sense to just use a Vec to store them anyway and binary search to get a specific diagnostic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to a BTreeMap. After looking a little into a Vec solution, I didn't like it as it would have mean every diagnostic recording a value would have to go through the Vec...

Copy link
Member

Choose a reason for hiding this comment

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

O(logn) is better than O(n), but for a large number of diagnostics idk if its a good idea to replace a hash map. While we currently only have a small number, I don't want to assume that will always be the case. Maybe LogDiagnosticsPlugin could store its own order? Sorting is a behavior inherent to the LogDiagnosticsPlugin so that makes sense to me.

The issue is keeping the order synced up / accounting for new diagnostics. Lol we could have a Diagnostic event. Ex: DiagnosticEvent::Added(DiagnosticId), DiagnosticEvent::Removed(DiagnosticId), etc

Copy link
Member Author

Choose a reason for hiding this comment

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

I looked into adding an event, seems not easy as adding a diagnostics can happen at app build time by mutating a resource, so the new diagnostic plugin would be responsible from calling the event (I think)
Keeping the order in LogDiagnosticsPlugin without events seemed not pretty
I added a BTreeSet of the DiagnosticId to Diagnostics that is updated when adding a diagnostics, and that is used for iterating orderedly

Self::print_diagnostic(diagnostic);
Self::log_diagnostic(diagnostic);
}
}
}
}

pub fn print_diagnostics_debug_system(
mut state: ResMut<PrintDiagnosticsState>,
fn log_diagnostics_debug_system(
mut state: ResMut<LogDiagnosticsState>,
time: Res<Time>,
diagnostics: Res<Diagnostics>,
) {
if state.timer.tick(time.delta_seconds()).finished() {
println!("Diagnostics (Debug):");
println!("{}", "-".repeat(93));
if let Some(ref filter) = state.filter {
for diagnostic in filter.iter().map(|id| diagnostics.get(*id).unwrap()) {
println!("{:#?}\n", diagnostic);
debug!("{:#?}\n", diagnostic);
}
} else {
for diagnostic in diagnostics.iter() {
println!("{:#?}\n", diagnostic);
debug!("{:#?}\n", diagnostic);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions examples/3d/spawner.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use bevy::{
diagnostic::{FrameTimeDiagnosticsPlugin, PrintDiagnosticsPlugin},
diagnostic::{FrameTimeDiagnosticsPlugin, LogDiagnosticsPlugin},
prelude::*,
};
use rand::{rngs::StdRng, Rng, SeedableRng};
Expand All @@ -12,7 +12,7 @@ fn main() {
App::build()
.add_plugins(DefaultPlugins)
.add_plugin(FrameTimeDiagnosticsPlugin::default())
.add_plugin(PrintDiagnosticsPlugin::default())
.add_plugin(LogDiagnosticsPlugin::default())
.add_startup_system(setup.system())
.add_system(move_cubes.system())
.run();
Expand Down
2 changes: 1 addition & 1 deletion examples/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ Example | File | Description
Example | File | Description
--- | --- | ---
`custom_diagnostic` | [`diagnostics/custom_diagnostic.rs`](./diagnostics/custom_diagnostic.rs) | Shows how to create a custom diagnostic
`print_diagnostics` | [`diagnostics/print_diagnostics.rs`](./diagnostics/print_diagnostics.rs) | Add a plugin that prints diagnostics to the console
`log_diagnostics` | [`diagnostics/log_diagnostics.rs`](./diagnostics/log_diagnostics.rs) | Add a plugin that logs diagnostics to the console

## ECS (Entity Component System)

Expand Down
2 changes: 1 addition & 1 deletion examples/app/plugin_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fn main() {
// .add_plugins_with(HelloWorldPlugins, |group| {
// group
// .disable::<PrintWorldPlugin>()
// .add_before::<PrintHelloPlugin, _>(bevy::diagnostic::PrintDiagnosticsPlugin::default())
// .add_before::<PrintHelloPlugin, _>(bevy::diagnostic::LogDiagnosticsPlugin::default())
// })
.run();
}
Expand Down
4 changes: 2 additions & 2 deletions examples/diagnostics/custom_diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use bevy::{
diagnostic::{Diagnostic, DiagnosticId, Diagnostics, PrintDiagnosticsPlugin},
diagnostic::{Diagnostic, DiagnosticId, Diagnostics, LogDiagnosticsPlugin},
prelude::*,
};

Expand All @@ -8,7 +8,7 @@ fn main() {
App::build()
.add_plugins(DefaultPlugins)
// The "print diagnostics" plugin is optional. It just visualizes our diagnostics in the console
.add_plugin(PrintDiagnosticsPlugin::default())
.add_plugin(LogDiagnosticsPlugin::default())
.add_startup_system(setup_diagnostic_system.system())
.add_system(my_system.system())
.run();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use bevy::{
diagnostic::{FrameTimeDiagnosticsPlugin, PrintDiagnosticsPlugin},
diagnostic::{FrameTimeDiagnosticsPlugin, LogDiagnosticsPlugin},
prelude::*,
};

Expand All @@ -9,9 +9,13 @@ fn main() {
// Adds frame time diagnostics
.add_plugin(FrameTimeDiagnosticsPlugin::default())
// Adds a system that prints diagnostics to the console
.add_plugin(PrintDiagnosticsPlugin::default())
.add_plugin(LogDiagnosticsPlugin::default())
// Any plugin can register diagnostics
// Uncomment this to add some render resource diagnostics:
// .add_plugin(bevy::wgpu::diagnostic::WgpuResourceDiagnosticsPlugin::default())
// Uncomment this to add an entity count diagnostics:
// .add_plugin(bevy::diagnostic::EntityCountDiagnosticsPlugin::default())
// Uncomment this to add an asset count diagnostics:
// .add_plugin(bevy::asset::diagnostic::AssetCountDiagnosticsPlugin::<Texture>::default())
.run();
}