Use precompiled headers for libopenrct2 with MSVC

[janisozaur]

Reduces compilation with msvc by several minutes.

There's more where this came from - in future PRs

[janisozaur]

Would like to hear from @duncanspumpkin how it works for you

[tupaschoal]

If we're doing something for msvc only, I suspect modules might also be a good option, I think it's the first compiler to fully support it and there were claims of impressive speedups shared by Microsoft at CppCon2021

PS: note that it's a bit orthogonal to this PR, as my first try on modules would be to go after the STL includes, as it would require less effort

[IntelOrca]

I don't like precompiled headers... can we instead just work on reducing the use of common.h?

[ZehMatt]

I don't like precompiled headers... can we instead just work on reducing the use of common.h?

Whats wrong with precompiled headers? Also common.h is almost empty now, won't improve a lot.

[janisozaur]

I was under impression using MSVC requires the mindset to yearn for PCHs. Like Matt said, it's not the common.h that is expensive, but ride.h and others. This gives significant speed boost in CI and I'm sure on your machine as well - so why not use it? I would also implement that for Linux, but we already have much better solution in form of ccache - I played with clcache and it hit limitations with how the project is set up (/Zi), so this is next best thing. I have already spent a lot of time reducing both header inclusion and their heft - while this work is never 'done', we're at a point where it offers limited gains compared to method like this one.

Time samples from before this PR (https://github.com/OpenRCT2/OpenRCT2/runs/4323822849?check_suite_focus=true): x86: 13m 49s amd64: 15m 15s

With this PR (https://github.com/OpenRCT2/OpenRCT2/runs/4324965912?check_suite_focus=true): x86: 10m 13s (3m 36s or 26% shorter) amd64: 11m 18s (3m 57s or 26% shorter)

[IntelOrca]

I was under impression using MSVC requires the mindset to yearn for PCHs.

I generally find them a nuisance. They require some odd requirements sometimes like needing it to be the first include in a source file.

[ZehMatt]

I was under impression using MSVC requires the mindset to yearn for PCHs.

I generally find them a nuisance. They require some odd requirements sometimes like needing it to be the first include in a source file.

Yeah the include requirement is a bit annoying but that is covered by adding it to the forced includes so you wouldn't actually need to include it manually. We should be a bit careful about what we put in there still.

[Broxzier]

By using a precompiled header and forcing all source files to include it, you also force a full recompilation of the project when modifying any of the header files that get included. How does it compare to build times when making an adjustment to one of the big headers?

[YJSoft]

First build / After cleanup build result Compile order is nopch -> pch / pch -> nopch Tested env:

CPU : Ryzen 7 5800X
Mem : 32GB
with NVMe SSD

---

After clone

Without precompiled header : 2m25s / 1m29s
With precompiled header : 1m44s / 1m16s

After small code change(src/openrct2/interface/Fonts.cpp file)

Without precompiled header : 38s / 38s
With precompiled header : 29s / 30s

After small(but affect lots of file) code change(src/openrct2/ride/Ride.h file)

Without precompiled header : 1m38s / 1m38s
With precompiled header : 1m9s / 1m7s

By using a precompiled header and forcing all source files to include it, you also force a full recompilation of the project when modifying any of the header files that get included. How does it compare to build times when making an adjustment to one of the big headers?

Yes. It recompiles almost every code. but It's same without precompiled header.

[janisozaur]

Yes. It recompiles almost every code. but It's same without precompiled header.

It's probably a bit different - build systems track inclusion of files and by modifying one of the files in PCH you force rebuild of all things that include it - so possibly a superset of what you would need to rebuild otherwise. That said Ride.h is included in 402 TUs, FileSystem.hpp in 433, Object.h in 491, so virtually in all of the project

[duncanspumpkin]

I've started breaking up ride.h so it's not included as much. Presumably that will reduce the benefits of this?

[janisozaur]

We shall see. As it won't affect your work and provides tangible improvement now, can we meet this? Also @duncanspumpkin is there improvement in your side as well?

[duncanspumpkin]

We shall see. As it won't affect your work and provides tangible improvement now, can we meet this? Also @duncanspumpkin is there improvement in your side as well?

I'll give it a whirl tomorrow.

[tupaschoal]

Have you tried modules (with c++20)? Seems to give very significant boosts for msvc, though not yet available elsewhere.

I might play with the c++20 branch to get this closer

[janisozaur]

No, I haven't. Didn't you say it would limit us to MSVC only? Also got a link to that cppcon talk?

[tupaschoal]

No, I haven't. Didn't you say it would limit us to MSVC only?

Yeah, a few ifdefs here and there, but once modules were widespread, it would be an automatic cleanup.

Also got a link to that cppcon talk?

https://youtu.be/9OWGgkuyFV8

[janisozaur]

	develop	this PR	delta
x86	19m 38s	13m 29s	-6m 9s
amd64	17m 39s	11m 51s	-5m 48s

[janisozaur]

This was put on hold while we try getting ccache working: see https://github.com/OpenRCT2/OpenRCT2/issues/16895

[tupaschoal]

Does the update here mean we got it to work with ccache? :D

[rik-smeets]

What's the status of this, @janisozaur ?

[janisozaur]

I've seen @tupaschoal try ccache for Windows. This PR does not do that. This is pretty much still gated by https://github.com/OpenRCT2/OpenRCT2/pull/15997#issuecomment-979500495, but would love to get this in for the massive gains it actually provides.

[github-actions[bot]]

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

[ZehMatt]

What should we do about this one?

[IntelOrca]

What should we do about this one?

Look at a header include graph and reduce large ones, or split large ones into smaller ones.

[ZehMatt]

What should we do about this one?

Look at a header include graph and reduce large ones, or split large ones into smaller ones.

This is not the same as precompiled headers and you won't really get close to that by just reducing includes. Also it would be interesting to see how much it helps with LTCG off now where it was previously always enabled.

[IntelOrca]

@ZehMatt well if you say so. I am not a fan of precompiled headers, I have only had bad experiences with them.

[duncanspumpkin]

I'm not a fan of them either but my issues with them are more a problem if every platform was using them.

[ZehMatt]

I'm fine without them, I think we can close this PR, no need for it to collect dust.

[janisozaur]

@IntelOrca what kind of bad experiences do you mean?

[733737]

windows builds take 2 or 3 times as long as linux builds. I think there would be much less people complaining about a broken launcher if they weren't waiting for windows builds to compile for 20 minutes or longer