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] - simple tool to compare traces between executions #4628

Closed
wants to merge 16 commits into from

Conversation

mockersf
Copy link
Member

Objective

  • Have an easy way to compare spans between executions

Solution

  • Add a tool to compare spans from chrome traces
> cargo run --release  -p spancmp -- --help
   Compiling spancmp v0.1.0
    Finished release [optimized] target(s) in 1.10s
     Running `target/release/spancmp --help`
spancmp

USAGE:
    spancmp [OPTIONS] <TRACE> [SECOND_TRACE]

ARGS:
    <TRACE>
    <SECOND_TRACE>

OPTIONS:
    -h, --help                     Print help information
    -p, --pattern <PATTERN>        Filter spans by name matching the pattern
    -t, --threshold <THRESHOLD>    Filter spans that have an average shorther than the threshold
                                   [default: 0]

for each span, it will display the count, minimum duration, average duration and max duration. It can be filtered by a pattern on the span name or by a minimum average duration.

just displaying a trace
Screenshot 2022-04-28 at 21 56 21

comparing two traces
Screenshot 2022-04-28 at 21 56 55

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Apr 28, 2022
@superdump
Copy link
Contributor

  1. Ooooooo
  2. Aaaaahhh
  3. ???
  4. Profit!
  5. Maybe a terminal UI and some more and selectable metrics and comparisons :)

@hymm
Copy link
Contributor

hymm commented Apr 28, 2022

This is something that tracy does too (most of it at least). This seems like a nice command line tool, but what is the argument for it to be included in bevy?
image

@superdump
Copy link
Contributor

Wait what? How did I miss that Tracy had that?? Awesome!

@alice-i-cecile alice-i-cecile added C-Performance A change motivated by improving speed, memory usage or compile times A-Log C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Apr 29, 2022
@mockersf
Copy link
Member Author

mockersf commented Apr 29, 2022

This is something that tracy does too (most of it at least). This seems like a nice command line tool, but what is the argument for it to be included in bevy?

While I like tracy, it's an extra tool to install and more complicated to use.

I would like to promote more for PR the ability to compare spans before/after, but that's currently something that need a somewhat complicated setup. This tool was made in the idea of critcmp. Somewhere in the future, I would like a bot that post results for example trace comparison automatically to PRs, but it could be done manually for now.

I added it in Bevy because it's not a general chrome trace comparison tool, it only supports the traces emitted by Bevy, and we can make it evolve based on the stats we want to monitor.

@alice-i-cecile
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Apr 30, 2022
@bors
Copy link
Contributor

bors bot commented Apr 30, 2022

try

Build failed:

@superdump
Copy link
Contributor

This is something that tracy does too (most of it at least). This seems like a nice command line tool, but what is the argument for it to be included in bevy?

While I like tracy, it's an extra tool to install and more complicated to use.

Honestly, I agree. It's already clunky to switch branches to run the profiles. The Tracy workflow then involves clicking around in a GUI to open the traces, find the relevant systems in the two traces to compare, noting down the statistics to write into a discussion / issue / PR / whatever. If I could just capture the two profiles and then run a command that spits out the information I want, that's going to be a faster workflow.

I added it in Bevy because it's not a general chrome trace comparison tool, it only supports the traces emitted by Bevy, and we can make it evolve based on the stats we want to monitor.

I'm in. And you know I'll make use of this so...

@alice-i-cecile
Copy link
Member

I like the justification here. I'm on board with the design direction here, although I haven't reviewed the code in depth yet.

Comment on lines 19 to 24
/// name
name: String,
/// phase
ph: String,
/// timestamp
ts: f32,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// name
name: String,
/// phase
ph: String,
/// timestamp
ts: f32,
name: String,
phase: String,
timestamp: f32,

and fix references. I see no reason to shorten the member names, nor to document with what the member names are/should be.

Copy link
Member Author

@mockersf mockersf May 3, 2022

Choose a reason for hiding this comment

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

The fields have the same name as in the json for deserialization. They can be annotated with the serde annotation to rename them, but I tend to prefer to stick to the same name as the json being deserialised when not part of the public api

tools/spancmp/src/main.rs Outdated Show resolved Hide resolved
tools/spancmp/src/main.rs Outdated Show resolved Hide resolved
second_trace: Option<String>,
}

const MARGIN_PERCENT: f32 = 0.02;
Copy link
Contributor

Choose a reason for hiding this comment

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

I just hacked this in. It should probably be a CLI argument with a default. :) Later we can base it on the standard deviation of the distributions or something statistically sensible.

tools/spancmp/src/main.rs Outdated Show resolved Hide resolved
@mockersf
Copy link
Member Author

mockersf commented May 3, 2022

new look thanks to @superdump
Screenshot 2022-05-04 at 01 36 18

@alice-i-cecile
Copy link
Member

alice-i-cecile commented May 4, 2022

Can we use short names for the systems here? We already have a tool for this (get_short_name in bevy_reflect), that I refactored in #4299

@mockersf
Copy link
Member Author

mockersf commented May 4, 2022

Almost, but not exactly: here the name is not directly a rust type name, it's a string, and we want to keep part of that string.

It's also useful to keep the full type to be able to filter all spans from bevy_render for example.

System names in spans are not easy to read, but most of the info in their name is useful. That's also painful when using other traces tool 🙁

@mockersf
Copy link
Member Author

mockersf commented May 4, 2022

@alice-i-cecile in fact, it's possible with some regex

I added a flag --short that will simplify system names. It has issues on some system names, we'll see once your updated version is merged if it's better
Screenshot 2022-05-04 at 03 07 22

@mockersf
Copy link
Member Author

mockersf commented May 4, 2022

now with everything aligned and time units thanks to @superdump
Screenshot 2022-05-04 at 21 06 20

Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

I think we should merge this then continue iterating on it. :)

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 5, 2022
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I'm in agreement with Rob; this looks fine to me and I agree with the direction.

Because there's no user impact I'm happy to take a merge and iterate approach.

@superdump
Copy link
Contributor

bors r+

bors bot pushed a commit that referenced this pull request May 6, 2022
# Objective

- Have an easy way to compare spans between executions

## Solution

- Add a tool to compare spans from chrome traces

```bash
> cargo run --release  -p spancmp -- --help
   Compiling spancmp v0.1.0
    Finished release [optimized] target(s) in 1.10s
     Running `target/release/spancmp --help`
spancmp

USAGE:
    spancmp [OPTIONS] <TRACE> [SECOND_TRACE]

ARGS:
    <TRACE>
    <SECOND_TRACE>

OPTIONS:
    -h, --help                     Print help information
    -p, --pattern <PATTERN>        Filter spans by name matching the pattern
    -t, --threshold <THRESHOLD>    Filter spans that have an average shorther than the threshold
                                   [default: 0]
```

for each span, it will display the count, minimum duration, average duration and max duration. It can be filtered by a pattern on the span name or by a minimum average duration.

just displaying a trace
![Screenshot 2022-04-28 at 21 56 21](https://user-images.githubusercontent.com/8672791/165835310-f465c6f2-9e6b-4808-803e-884b06e49292.png)

comparing two traces
![Screenshot 2022-04-28 at 21 56 55](https://user-images.githubusercontent.com/8672791/165835353-097d266b-a70c-41b8-a8c1-27804011dc97.png)



Co-authored-by: Robert Swain <[email protected]>
@bors
Copy link
Contributor

bors bot commented May 6, 2022

Build failed:

@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request May 6, 2022
# Objective

- Have an easy way to compare spans between executions

## Solution

- Add a tool to compare spans from chrome traces

```bash
> cargo run --release  -p spancmp -- --help
   Compiling spancmp v0.1.0
    Finished release [optimized] target(s) in 1.10s
     Running `target/release/spancmp --help`
spancmp

USAGE:
    spancmp [OPTIONS] <TRACE> [SECOND_TRACE]

ARGS:
    <TRACE>
    <SECOND_TRACE>

OPTIONS:
    -h, --help                     Print help information
    -p, --pattern <PATTERN>        Filter spans by name matching the pattern
    -t, --threshold <THRESHOLD>    Filter spans that have an average shorther than the threshold
                                   [default: 0]
```

for each span, it will display the count, minimum duration, average duration and max duration. It can be filtered by a pattern on the span name or by a minimum average duration.

just displaying a trace
![Screenshot 2022-04-28 at 21 56 21](https://user-images.githubusercontent.com/8672791/165835310-f465c6f2-9e6b-4808-803e-884b06e49292.png)

comparing two traces
![Screenshot 2022-04-28 at 21 56 55](https://user-images.githubusercontent.com/8672791/165835353-097d266b-a70c-41b8-a8c1-27804011dc97.png)



Co-authored-by: Robert Swain <[email protected]>
@bors bors bot changed the title simple tool to compare traces between executions [Merged by Bors] - simple tool to compare traces between executions May 6, 2022
@bors bors bot closed this May 6, 2022
robtfm pushed a commit to robtfm/bevy that referenced this pull request May 10, 2022
# Objective

- Have an easy way to compare spans between executions

## Solution

- Add a tool to compare spans from chrome traces

```bash
> cargo run --release  -p spancmp -- --help
   Compiling spancmp v0.1.0
    Finished release [optimized] target(s) in 1.10s
     Running `target/release/spancmp --help`
spancmp

USAGE:
    spancmp [OPTIONS] <TRACE> [SECOND_TRACE]

ARGS:
    <TRACE>
    <SECOND_TRACE>

OPTIONS:
    -h, --help                     Print help information
    -p, --pattern <PATTERN>        Filter spans by name matching the pattern
    -t, --threshold <THRESHOLD>    Filter spans that have an average shorther than the threshold
                                   [default: 0]
```

for each span, it will display the count, minimum duration, average duration and max duration. It can be filtered by a pattern on the span name or by a minimum average duration.

just displaying a trace
![Screenshot 2022-04-28 at 21 56 21](https://user-images.githubusercontent.com/8672791/165835310-f465c6f2-9e6b-4808-803e-884b06e49292.png)

comparing two traces
![Screenshot 2022-04-28 at 21 56 55](https://user-images.githubusercontent.com/8672791/165835353-097d266b-a70c-41b8-a8c1-27804011dc97.png)



Co-authored-by: Robert Swain <[email protected]>
exjam pushed a commit to exjam/bevy that referenced this pull request May 22, 2022
# Objective

- Have an easy way to compare spans between executions

## Solution

- Add a tool to compare spans from chrome traces

```bash
> cargo run --release  -p spancmp -- --help
   Compiling spancmp v0.1.0
    Finished release [optimized] target(s) in 1.10s
     Running `target/release/spancmp --help`
spancmp

USAGE:
    spancmp [OPTIONS] <TRACE> [SECOND_TRACE]

ARGS:
    <TRACE>
    <SECOND_TRACE>

OPTIONS:
    -h, --help                     Print help information
    -p, --pattern <PATTERN>        Filter spans by name matching the pattern
    -t, --threshold <THRESHOLD>    Filter spans that have an average shorther than the threshold
                                   [default: 0]
```

for each span, it will display the count, minimum duration, average duration and max duration. It can be filtered by a pattern on the span name or by a minimum average duration.

just displaying a trace
![Screenshot 2022-04-28 at 21 56 21](https://user-images.githubusercontent.com/8672791/165835310-f465c6f2-9e6b-4808-803e-884b06e49292.png)

comparing two traces
![Screenshot 2022-04-28 at 21 56 55](https://user-images.githubusercontent.com/8672791/165835353-097d266b-a70c-41b8-a8c1-27804011dc97.png)



Co-authored-by: Robert Swain <[email protected]>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Have an easy way to compare spans between executions

## Solution

- Add a tool to compare spans from chrome traces

```bash
> cargo run --release  -p spancmp -- --help
   Compiling spancmp v0.1.0
    Finished release [optimized] target(s) in 1.10s
     Running `target/release/spancmp --help`
spancmp

USAGE:
    spancmp [OPTIONS] <TRACE> [SECOND_TRACE]

ARGS:
    <TRACE>
    <SECOND_TRACE>

OPTIONS:
    -h, --help                     Print help information
    -p, --pattern <PATTERN>        Filter spans by name matching the pattern
    -t, --threshold <THRESHOLD>    Filter spans that have an average shorther than the threshold
                                   [default: 0]
```

for each span, it will display the count, minimum duration, average duration and max duration. It can be filtered by a pattern on the span name or by a minimum average duration.

just displaying a trace
![Screenshot 2022-04-28 at 21 56 21](https://user-images.githubusercontent.com/8672791/165835310-f465c6f2-9e6b-4808-803e-884b06e49292.png)

comparing two traces
![Screenshot 2022-04-28 at 21 56 55](https://user-images.githubusercontent.com/8672791/165835353-097d266b-a70c-41b8-a8c1-27804011dc97.png)



Co-authored-by: Robert Swain <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants