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

[Merged by Bors] - Automatic System Spans #2033

Closed
wants to merge 3 commits into from

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Apr 27, 2021

As mentioned in #2025 (comment), systems used to have spans by default.

  • add spans by default for every system executed
  • create folder if missing for feature wgpu_trace

@Moxinilian Moxinilian added A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible labels Apr 27, 2021
Comment on lines 49 to 57
#[cfg(feature = "trace")]
let trace_path = Some(std::path::Path::new("wgpu_trace"));
let trace_path = {
let path = std::path::Path::new("wgpu_trace");
// ignore potential error, wgpu will log it
let _ = std::fs::create_dir(path);
Some(path)
};
#[cfg(not(feature = "trace"))]
let trace_path = None;
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole block could be cobined into a if cfg!(...) expression which would be clearer imo.

let trace_path = if cfg!(feature = "trace") {
    let path = std::path::Path::new("wgpu_trace");
    // ignore potential error, wgpu will log it
    let _ = std::fs::create_dir(path);
    Some(path)
} else {
    None
};

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 prefer the current way as I feel it's less work for the compiler.

But then this should be very simple to compile away... 🤷

@cart
Copy link
Member

cart commented Apr 27, 2021

Just wanted to voice my appreciation for how often you quickly pick up and implement work that "needs to be done" (and its generally exactly what we need). You've done a lot to improve the quality of Bevy in a ton of different areas and its awesome to see.

Co-Authored-By: TehPers <[email protected]>
@cart
Copy link
Member

cart commented Apr 28, 2021

Awesome. Chrome tracing now shows systems again!
image

@cart
Copy link
Member

cart commented Apr 28, 2021

bors r+

bors bot pushed a commit that referenced this pull request Apr 28, 2021
As mentioned in #2025 (comment), systems used to have spans by default.

* add spans by default for every system executed
* create folder if missing for feature `wgpu_trace`
@bors bors bot changed the title Automatic System Spans [Merged by Bors] - Automatic System Spans Apr 28, 2021
@bors bors bot closed this Apr 28, 2021
NiklasEi pushed a commit to NiklasEi/bevy that referenced this pull request May 1, 2021
As mentioned in bevyengine#2025 (comment), systems used to have spans by default.

* add spans by default for every system executed
* create folder if missing for feature `wgpu_trace`
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
As mentioned in bevyengine#2025 (comment), systems used to have spans by default.

* add spans by default for every system executed
* create folder if missing for feature `wgpu_trace`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants