-
Notifications
You must be signed in to change notification settings - Fork 654
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
Add Tracy Profiler Support #757
Add Tracy Profiler Support #757
Conversation
f7f2080
to
3e2b3be
Compare
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.
Is the concurrency stuff in the *.yml files related to adding tracy?
Besides that, I'm not familiar with tracy. How would I get that nice little tracy window?
{ | ||
average += v; | ||
} | ||
average /= c.second.size(); |
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.
Maybe: float average = std::accumulate( c.second.begin(), c.second.end(), 0.0f ) / c.second.size();
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.
Nice, yeah a bit more concise
The concurrency stuff isn't related, I can make a small PR for these changes if needed. Just threw it in here as I had lots of jobs running and not automatically cancelling outdated ones. To use Tracy you should be able to run './scripts/tracy.py' and it will build and run the window. I have only tested this on Linux and you do need to install system libraries. I can give it a go on windows at some point this week and make sure it works. It is likely that it won't work first time |
And you're right, tracy on windows is not supported.
You can keep it in, if you like. I just wanted to make that I didn't miss something here. |
I'm pretty sure tracy works on Windows. I'm using it in one of my projects and I mainly develop on windows ;) |
Tracy is supported. It is likely my script doesn't work on windows - Yet! The script was meant as a build helper as you need the same Tracy version as TracyClient version used in the app. I've only had chance to get this helper working on Linux but i should have some free time tonight to test it out on windows. The latest Tracy download from GitHub should also work (again not tested this as I was focused on the script) |
The build script for Tracy doesn't work for me in Linux sadly:
|
@asuessenbach sorry for the delay on this. Each time I have thought I would have time I've had a distraction I'm going to pin the submodule version to v9.1.0 https://github.com/wolfpld/tracy/tree/v0.9.1 This would make the versions compatible. I think I am currently on v.9.2.0 (using main) Will also try the Windows build now with the script @gary-sweet what is the version of your capstone library? I believe that build issue is coming from a missing dependency. The first printout at the top of the script is some install dependencies. It may work after building those |
3e2b3be
to
1b27c10
Compare
Wow, the Windows build is much more involved. Pinning the Tracy version to v0.9.1 makes our client compatible with the profiler Added some download instructions to the script I would download automatically but as the release page packages Tracy in a 7zip archive I feel like it is best to let the user do this After rebuilding samples I get this |
1b27c10
to
771633f
Compare
Moved concurrency changes to another PR |
f3f89fc
to
54e953a
Compare
3c28a94
to
64faa4e
Compare
Picked this branch back up as the Tracy support does work. This branch got stalled on that fact that Tracy is hard to build for different platforms - this is a known limitation of the project. I have created this pr in an attempt to help improve Tracy in this area. Unfortunately you will not be able to use this branch to build Tracy due to the strict protocol constraints. If you would like to test this PR you will need to install Tracy from the release page or by following the Tracy.pdf guide. If you are using version of ubuntu that does not include capstone 4+ you will need to manually build and install capstone 4+ in your distro for Tracy to build.
For now I have only instrumented up the GLTF Loader. We do not need to instrument all functions with Tracy. It is useful to track the performance of high level systems such as scene iterations or multi threaded scenarios. I have tested on Ubuntu 23.04, Windows 11 and Android. Tracy is disabled for Android for now as there are some networking considerations (I will be addressing any CI issues over the next few days, There are likely going to be some nuances between actual machines and github runners) |
Unfortunately, I get an "Incompatible protocol" error, again: I just downloaded the release v0.10 from https://github.com/wolfpld/tracy/tags, started the Tracy.exe and got this. |
@@ -21,39 +21,12 @@ | |||
#include <stdexcept> | |||
#include <string> | |||
|
|||
#include "core/util/error.hpp" |
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.
Why do you separate error.hpp for the other headers?
@@ -29,6 +29,8 @@ VKBP_DISABLE_WARNINGS() | |||
#include <glm/gtc/type_ptr.hpp> | |||
VKBP_ENABLE_WARNINGS() | |||
|
|||
#include <core/util/profiling.hpp> |
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.
Why do you separate profiling.hpp from the other headers below?
Hey @asuessenbach, this branch is pinned to v0.10 can you confirm that you updated your submodules before building? Before this branch was v0.9.1 so it may be that you still have that version checked out |
Works fine for me 👍🏻. Will try to do a proper review this weekend. |
@tomadamatkinson You're right, I somehow missed to get the check out the right version. Works now! |
@tomadamatkinson: Could you fix the the merge conflicts? Would' love to see this merged, so we can add additional functionality based on your work. If you can't find the time, I'll happily try to help :) |
@SaschaWillems ill take a look! |
Android fixes document tracy
1e16cb9
45b687d
to
1e16cb9
Compare
Rebased and tested on linux. To work with android devices i believe we need to port forward with adb and also give the application local network permissions. I have not tested this in this PR. On desktop this should work fine out of the box. Works with Tracy version 0.10. When Tracy has a new official release with the CMake build system i helped with we can bump the submodule and point to that allowing users to seamlessly build the profiler. I cannot test on Windows or Mac today but may be able too in the near future @SaschaWillems let me know if there are any tweaks you want adding to this |
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.
Thank you very much. Worked out of the box for me on Windows with the Tracy 0.10 binary release 👍🏻
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.
Awesome, works here.
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.
Doesn't break anything for me
Merging - 3 approvals |
Description
Adds Tracy Profiler support. This will allow samples to show the stats and usage that they want to show to users without needing to clutter the Sample GUI
This can be extended in the future to instrument up core parts of the framework and samples that we want to draw the users attention too. It can also be used to show how multi-threading can be used with Vulkan
For now I have hooked this into our stats system so that we can show more stat information to users without worrying about cluttering the main GUI. We can still use the main sample GUI to show stats aswell!! I also added support for VMA Memory Budgets to show heap usage
We can use this to track other useful information such as the amount of vertices we expect to render for a given frame or the amount of geometry in the scene graph
Tracy also has Vulkan support however I have not played around with this yet
String names for VkMemoryTypeFlags are taking from vulkan.hpp instead of adding to the existing vkb::to_string() set
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
Sample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: