Skip to content

[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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aliendroid1
Copy link

@aliendroid1 aliendroid1 commented Jun 19, 2025

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

@aliendroid1 aliendroid1 changed the title Remove obsolete profile library and builds [CMAKE][ZH] Remove obsolete profile library and builds Jun 19, 2025
@Mauller Mauller added Build Anything related to building, compiling ZH Relates to Zero Hour labels Jun 23, 2025
@OmniBlade
Copy link

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.

@aliendroid1
Copy link
Author

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

@xezon
Copy link

xezon commented Jun 23, 2025

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.

@aliendroid1
Copy link
Author

Which in game diagnostics? If you're thinking of the commandmap debug utilities, those don't use this profile library

@xezon
Copy link

xezon commented Jun 24, 2025

For example it could render stalling functions directly in the game for very fast diagnostic.

@aliendroid1
Copy link
Author

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?

@xezon
Copy link

xezon commented Jun 24, 2025

I do not know if that is used. It was just a sample use case for the RTS_PROFILE in the future.

@aliendroid1
Copy link
Author

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?

@aliendroid1 aliendroid1 changed the title [CMAKE][ZH] Remove obsolete profile library and builds [CMAKE][ZH] Remove obsolete profile library Jun 24, 2025
@aliendroid1
Copy link
Author

I do not know if that is used. It was just a sample use case for the RTS_PROFILE in the future.

I've updated the pr and it no longer changes anything in cmakepresets or any .cmake file

@xezon
Copy link

xezon commented Jun 24, 2025

Macro aside, you're ok with removing the profile directory right?

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.

@aliendroid1
Copy link
Author

Macro aside, you're ok with removing the profile directory right?

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.

@xezon xezon added the Do Later Work on this later label Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Anything related to building, compiling Do Later Work on this later ZH Relates to Zero Hour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants