-
Notifications
You must be signed in to change notification settings - Fork 78
[CMAKE][ZH] Remove obsolete profile library #1084
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
base: main
Are you sure you want to change the base?
Conversation
I'm not that familiar with what modern profiling tools are available to us, but if there is agreement I'm happy to see the old profile library removed. The CI does also need updating in this PR to not try and build the profile presets though. |
Valgrind is a good one. There's also a performance analyzer built-in with visual studio |
I prefer not removing it until we work on performance and add a replacement. There actually is great value in having an ingame profiler that renders diagnostics directly in the game. I do not know the capabilities of "Profile", but would prefer we review this later when we actually do the performance investigations. Vote for close. |
Which in game diagnostics? If you're thinking of the commandmap debug utilities, those don't use this profile library |
For example it could render stalling functions directly in the game for very fast diagnostic. |
Which function is used for this and which header file is it declared in? |
I do not know if that is used. It was just a sample use case for the RTS_PROFILE in the future. |
Macro aside, you're ok with removing the profile directory right? |
40cdaad
to
7a6631b
Compare
I've updated the pr and it no longer changes anything in cmakepresets or any .cmake file |
I have not inspected the EA Profiler. Maybe it is useless. Maybe not. We are not doing any profiling right now, so it is not really a focus to look into that thing. There is no urgency to remove this, other than combining RTS_PROFILE and RTS_DEBUG into one CI target to reduce the amount of targets to build. |
Its not that much code to look into and it is important because its code bloat that adds dependencies and increases the surface area for bugs and incompatibilities. It is very obvious that it is useless. Its not a maybe. Our dependency graph and cmake build system setup is a rat's nest and I'm trying to fix that. Please take a look at it. It shouldn't take more than 5 minutes. If you check out my commits individually in order, you can see each step is very straightforward and non-controversial. |
It was only used twice, only in zero hour, and
even those uses were redundant.
Modern profiling tools such as Visual Studio's performance profiler and Valgrind can do much more without adding any dependencies