Skip to content

[CMAKE] Enable multithreaded compilation for VC6 builds #1169

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
merged 2 commits into from
Jul 2, 2025

Conversation

aliendroid1
Copy link

@aliendroid1 aliendroid1 commented Jun 22, 2025

Created a custom cmake toolchain file for using the vc6 compiler with the ninja generator

Switching from nmake to multithreaded ninja reduces compile times by an order of magnitude

@Mauller
Copy link

Mauller commented Jun 22, 2025

I would make this as an alternative build method instead of a replacement since we still need PDB files when debugging crashes.

@aliendroid1
Copy link
Author

I would make this as an alternative build method instead of a replacement since we still need PDB files when debugging crashes.

It still has embedded debug information

@xezon xezon requested a review from OmniBlade June 22, 2025 17:58
@xezon
Copy link

xezon commented Jun 22, 2025

@OmniBlade What are your thoughts on this?

@Mauller Mauller added the Build Anything related to building, compiling label Jun 23, 2025
Copy link

@OmniBlade OmniBlade left a comment

Choose a reason for hiding this comment

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

There is a lot of none functional change in the json files which could do with being reverted. No averse to an autoformatted file, but it should be applied in a separate PR.

@OmniBlade
Copy link

Just checked in VSCode and the way its formatted now is the way it gets autoformatted there, so I think I'd need a good case made for changing how its formatted?

@aliendroid1 aliendroid1 force-pushed the vc6toolchain branch 2 times, most recently from 521936d to a04e60c Compare June 23, 2025 12:17
@aliendroid1 aliendroid1 requested a review from OmniBlade June 23, 2025 12:29
@aliendroid1 aliendroid1 force-pushed the vc6toolchain branch 2 times, most recently from b65c45a to a8e7920 Compare June 24, 2025 09:51
@aliendroid1 aliendroid1 marked this pull request as draft June 28, 2025 02:23
@aliendroid1
Copy link
Author

I would make this as an alternative build method instead of a replacement since we still need PDB files when debugging crashes.

It seems to create a pdb even if debug information format is set to null

@aliendroid1 aliendroid1 marked this pull request as ready for review June 28, 2025 04:43
@Caball009
Copy link

Combined with #1210 this provides a huge improvement for VC6 build times for me.

@xezon
Copy link

xezon commented Jun 29, 2025

All vc6 targets are failing with CMake Error: Error: generator : Ninja

How do we get this Ninja locally then?

@aliendroid1
Copy link
Author

aliendroid1 commented Jun 29, 2025

All vc6 targets are failing with CMake Error: Error: generator : Ninja

How do we get this Ninja locally then?

Maybe you don't have ninja in your environment variables or haven't installed it?
https://ninja-build.org/
I do plan to update it to ninja multiconfig in a follow up pr once this one is merged

@xezon
Copy link

xezon commented Jun 29, 2025

I do not have it yes and CI is also failing for it. Should this perhaps be a new preset?

@aliendroid1
Copy link
Author

I do not have it yes and CI is also failing for it. Should this perhaps be a new preset?

The ci actually doesn't fail for it. It is only failing because delete cache needs to be run once but if I change the binary directory to a new one then it passes. Just download ninja. Look at the bottom of the link I sent

@xezon
Copy link

xezon commented Jun 29, 2025

How do we delete build cache?

@aliendroid1
Copy link
Author

How do we delete build cache?

I just click delete cache and reconfigure under Project in visual studio

@xezon
Copy link

xezon commented Jun 30, 2025

So how do we fix the CI failing builds? And what can we do for users that do not yet use Ninja?

@aliendroid1
Copy link
Author

With #1210 the visual studio ninja will be used automatically when using cmake inside visual studio. Plus our wiki already tells people to install ninja. They can also add the path to their version of ninja to the environment variiable. I'll try see if detection of ninja can be done in a .cmake file

As for the CI, I would look into it except I don't have access to submit a pr that modifies the CI. We just need to update the cache it uses

@aliendroid1
Copy link
Author

With #1210 the visual studio ninja will be used automatically when using cmake inside visual studio. Plus our wiki already tells people to install ninja. They can also add the path to their version of ninja to the environment variiable. I'll try see if detection of ninja can be done in a .cmake file

As for the CI, I would look into it except I don't have access to submit a pr that modifies the CI. We just need to update the cache it uses

we could add something like "CMAKE_MAKE_PROGRAM": "C:/PROGRA1/MICROS2/2022/COMMUN1/Common7/IDE/COMMON1/MICROS~1/CMake/Ninja/ninja.exe" to cache variables but then that would force using the ninja in visual studio 2022

@aliendroid1
Copy link
Author

With #1210 the visual studio ninja will be used automatically when using cmake inside visual studio. Plus our wiki already tells people to install ninja. They can also add the path to their version of ninja to the environment variiable. I'll try see if detection of ninja can be done in a .cmake file

As for the CI, I would look into it except I don't have access to submit a pr that modifies the CI. We just need to update the cache it uses

nvm I do have access. I just happened to be using an access token that didn't have the right permissions

@aliendroid1 aliendroid1 force-pushed the vc6toolchain branch 2 times, most recently from 043dbd4 to f50ca09 Compare July 1, 2025 13:28
@aliendroid1
Copy link
Author

So how do we fix the CI failing builds? And what can we do for users that do not yet use Ninja?

CI is now fixed

Copy link

@OmniBlade OmniBlade left a comment

Choose a reason for hiding this comment

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

Looks good, works locally for me. Good spot on being able to limit the linker to one concurrent run, that did indeed fix the issue I had trying to link concurrently with VC6. Before merge I would like clarification on if the compiler pools are required or if you can just leave that unspecified and let ninja determine the max pool size.

With this there doesn't appear to be much difference between VC6 and win32 presets, perhaps they should be merged into a single preset now that will vary only based on the environment they are run in.

@xezon
Copy link

xezon commented Jul 2, 2025

This change is amazing. VC6 compile time is incredible. Under 1 minute on my system. Can we have the same compile speed for VS 2022 builds as well?

Fyi, to use, I downloaded Windows Ninja from https://ninja-build.org/, extracted the ninja.exe into a folder, put it into Path environment variable, and then reconfigured CMake for VC6 builds.

@xezon xezon added Major Severity: Minor < Major < Critical < Blocker Performance Is a performance concern labels Jul 2, 2025
@xezon xezon added this to the Code foundation build up milestone Jul 2, 2025
@xezon xezon merged commit 6f98469 into TheSuperHackers:main Jul 2, 2025
14 checks passed
@aliendroid1
Copy link
Author

This change is amazing. VC6 compile time is incredible. Under 1 minute on my system. Can we have the same compile speed for VS 2022 builds as well?

Fyi, to use, I downloaded Windows Ninja from https://ninja-build.org/, extracted the ninja.exe into a folder, put it into Path environment variable, and then reconfigured CMake for VC6 builds.

I'm actually quite puzzled on why vs 2022 builds are so much slower even though they also use ninja. My guess is that its due to the way thread safety is setup but I will investigate

@xezon
Copy link

xezon commented Jul 3, 2025

Hmm on my machine it uses MSBuild.exe I think.

@aliendroid1
Copy link
Author

Hmm on my machine it uses MSBuild.exe I think.

Do you set the generator to visual studio?

@xezon
Copy link

xezon commented Jul 3, 2025

Yes

@aliendroid1
Copy link
Author

Yes

Then that's why. For vc6 as well if I use the visual studio generator then it's significantly slower than with ninja. Ninja in general is a really fast build tool.

@aliendroid1
Copy link
Author

Looks good, works locally for me. Good spot on being able to limit the linker to one concurrent run, that did indeed fix the issue I had trying to link concurrently with VC6. Before merge I would like clarification on if the compiler pools are required or if you can just leave that unspecified and let ninja determine the max pool size.

With this there doesn't appear to be much difference between VC6 and win32 presets, perhaps they should be merged into a single preset now that will vary only based on the environment they are run in.

Most the stuff can be extractedd into a common preset for them to inherit from but if we make them a single preset then they would use the same binary directory and that would make force having to delete the entire build directory rebuild cache every time we want to switch from building one to the other

@aliendroid1 aliendroid1 deleted the vc6toolchain branch July 5, 2025 14:38
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 Major Severity: Minor < Major < Critical < Blocker Performance Is a performance concern
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants