-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Changes from 3 commits
9b00f26
20b8319
57332cc
0111837
3c4c3cf
d3fefc2
c69f3a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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); | ||
} | ||
} |
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; |
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() { | ||
count += 1.; | ||
} | ||
diagnostics.add_measurement(Self::ENTITY_COUNT, count); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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] | ||
pub struct LogDiagnosticsState { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is Maybe use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It has to be
It's public, just not user addressable... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we stop exporting the systems, then that error goes away. I'm not sure why we would be exporting the systems anyway? It was created as public in 704a742, which was in May. I suspect it's just an oversight. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
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>, | ||
pub 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() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Without filter, the diagnostics are stored in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to a There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) |
||
Self::print_diagnostic(diagnostic); | ||
Self::log_diagnostic(diagnostic); | ||
} | ||
} | ||
} | ||
} | ||
|
||
pub fn print_diagnostics_debug_system( | ||
mut state: ResMut<PrintDiagnosticsState>, | ||
pub 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); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
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)There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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