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] - docs: Add section about using Tracy for profiling #4534

Closed
wants to merge 10 commits into from

Conversation

superdump
Copy link
Contributor

@superdump superdump commented Apr 19, 2022

Objective

  • Document how to do profiling with Tracy

Solution

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Apr 19, 2022
@superdump superdump added C-Docs An addition or correction to our documentation C-Performance A change motivated by improving speed, memory usage or compile times and removed S-Needs-Triage This issue needs to be labelled labels Apr 19, 2022
docs/profiling.md Outdated Show resolved Hide resolved
docs/profiling.md Outdated Show resolved Hide resolved
docs/profiling.md Outdated Show resolved Hide resolved

There are binaries available for Windows, and installation / build instructions for other operating systems can be found in the [Tracy documentation PDF](https://github.com/wolfpld/tracy/releases/latest/download/tracy.pdf).

It has a command line capture tool that can be used for capturing with minimal disruption to the execution of graphical applications, saving the profile data to a file. It also has a GUI that can be used to inspect these profile files, or live capture them, though running the live capture on the same machine will be a competing graphical application, which may impact results. As such, @superdump tends to use the command line to for capturing, and the GUI tool for inspecting.
Copy link
Member

Choose a reason for hiding this comment

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

Please line break your .md paragraphs; it makes for cleaner git changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I disagree for text because then I end up re-joining and re-splitting all the lines whenever any words change, which creates a lot of noise. If they are on one line, in GitHub's UI only the changed words are highlighted. Plus none of the other existing paragraphs in this document are split across multiple lines. I won't resist on this point if you want us to consistently split paragraphs onto multiple lines in documentation though as I respect your ownership of docs more than my own opinions on it. :)


There are binaries available for Windows, and installation / build instructions for other operating systems can be found in the [Tracy documentation PDF](https://github.com/wolfpld/tracy/releases/latest/download/tracy.pdf).

It has a command line capture tool that can be used for capturing with minimal disruption to the execution of graphical applications, saving the profile data to a file. It also has a GUI that can be used to inspect these profile files, or live capture them, though running the live capture on the same machine will be a competing graphical application, which may impact results. As such, @superdump tends to use the command line to for capturing, and the GUI tool for inspecting.
Copy link
Member

Choose a reason for hiding this comment

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

What does "capturing" mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll clarify this, but it means 'receiving and recording profile data.' The Tracy tool is called 'capture'.

Copy link
Contributor Author

@superdump superdump Jul 4, 2022

Choose a reason for hiding this comment

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

I feel like nicopap's rewording already makes this much clearer so I left it as-is after their change was committed. Let me know if you think it needs further clarification.

docs/profiling.md Outdated Show resolved Hide resolved
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.

Some nits, but I'll block on the explicit mention of your username :)

I'm very glad to have this; seems extremely useful.

@laundmo
Copy link
Contributor

laundmo commented Jun 26, 2022

This is still blocked on the username mention i assume? i ended up referring people to this PR multiple times by now, and would really appreciate it taken care of soon.

@alice-i-cecile
Copy link
Member

Just needs the author (@superdump) to clean up a couple of the nits. @laundmo if you're feeling up for it feel free to make a competing version of this with the fixes and we can merge that ASAP.

@superdump
Copy link
Contributor Author

I can update it sometime before the end of the week.

@superdump
Copy link
Contributor Author

This now provides information that depends on #5182

@superdump
Copy link
Contributor Author

bors try

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

bors bot commented Jul 4, 2022

try

Build failed:

@superdump
Copy link
Contributor Author

bors try

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

bors bot commented Jul 4, 2022

try

Build failed:

@superdump
Copy link
Contributor Author

Pffft flakey link checker.

@mockersf
Copy link
Member

mockersf commented Jul 4, 2022

well...

@superdump
Copy link
Contributor Author

well...

* it's not the link checker, it's the markdown lint - it was removed in [[Merged by Bors] - Remove markdown dead link check #4839](https://github.com/bevyengine/bevy/pull/4839)

Aha. I looked at the big bold red ERRORs starting at the top and saw the failure to get a GitHub token and I thought it was only the dead link check in markdown that used that.

* it's not flakey when it's failing every time

Given other PRs were passing and I saw the GitHub token failure that I have seen so many times before, I figured it was temporarily failing. The boy who cried wolf. :)

* it's complaining that you used html tags for images

Righto, I missed those lines because despite being important information, they are not highlighted in red. 😄 I'll fix them.

@superdump
Copy link
Contributor Author

Tests pass when you read, see, understand, and fix the errors that they report! \o/

@alice-i-cecile alice-i-cecile requested a review from hymm July 4, 2022 12:36
Copy link
Contributor

@nicopap nicopap left a comment

Choose a reason for hiding this comment

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

I've used this recently as a reference for #4240. I think a future PR could add a couple things:

  • Add a section about comparing two traces with tracy
  • Add a link to the tracing-tracy crate here for example, especially the table at the end of the README with the version matrix, and state the version used in bevy, so that the reader can at a glance know which tracy version to use.

@laundmo
Copy link
Contributor

laundmo commented Jul 4, 2022

so that the reader can at a glance know which tracy version to use.

is it good to postpone that? speaking from experience it can lead to a lot of frustration...

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it A-Diagnostics Logging, crash handling, error reporting and performance analysis and removed C-Performance A change motivated by improving speed, memory usage or compile times labels Jul 4, 2022
@alice-i-cecile
Copy link
Member

It's better to keep PRs small. I'll spin those off into new issues.

bors r+

bors bot pushed a commit that referenced this pull request Jul 4, 2022
# Objective

- Document how to do profiling with Tracy

# Solution

- The documentation of setting `RUST_LOG=info` in order to capture `wgpu` spans depends on #5182
@bors bors bot changed the title docs: Add section about using Tracy for profiling [Merged by Bors] - docs: Add section about using Tracy for profiling Jul 4, 2022
@bors bors bot closed this Jul 4, 2022
inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022
# Objective

- Document how to do profiling with Tracy

# Solution

- The documentation of setting `RUST_LOG=info` in order to capture `wgpu` spans depends on bevyengine#5182
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

- Document how to do profiling with Tracy

# Solution

- The documentation of setting `RUST_LOG=info` in order to capture `wgpu` spans depends on bevyengine#5182
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Document how to do profiling with Tracy

# Solution

- The documentation of setting `RUST_LOG=info` in order to capture `wgpu` spans depends on bevyengine#5182
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Docs An addition or correction to our documentation 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.

6 participants