-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[NOSQUASH] Precompiled headers support #13355
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
Conversation
I'd be worried the additional fragility in the process is not warranted by a 17% compilation time decrease. |
I tested this (8812b79) with 3 build directories (warm-up, PCH disabled, and PCH enabled) on my system (Ryzen 7 5800X). Here is time comparison:
Time is in seconds. Time diff is percentage of “no PCH.”
Corresponds to −17% “Real, diff” (Clean build) in my format. Which is just a bit better than what I get. Here are some details: Without PCH
With PCH
ConclusionWhile single source file updates are dominated by link time, there is a clear speedup for header updates which tend to require massive recompilation, and for full rebuilds too. |
It's optional and marked as experimental. And CI doesn't use it, so missing includes and similar should usually be found. Thanks for testing, @numberZero! |
Which may very well happen without any PCH because e.g. today, |
@Desour rebase needed |
b6f2146
to
21c0467
Compare
0d5ce91
to
6e27fee
Compare
Rebased and comments addressed. |
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.
Works for me but needs a rebase.
f54cd79
to
bad27f4
Compare
Precompiled headers don't work if we're not a pure C++ project.
Note: the <filesystem> header is not included in the default precompiled_headers.txt, because we don't use it yet, and it might be big
bad27f4
to
7910a25
Compare
Rebased and squashed. Will merge soonish. |
Recent CMake has a
target_precompile_headers
function that makes using precompiled headers pretty trivial.(For comparison, see #8643. (cc @numberZero))
Goal: Faster compilation, climate protection, less turbine noise.
1st half of commit:
Moves sha256.c to lib/. This is needed because precompiled headers don't work (assume C) if we're mixing C and C++.
2nd half of commit:
Precompiled header option. (Requires new enough cmake.)
Which headers are precompiled:
src/precompiled_headers.txt
). The user can configure this.To do
This PR is a Ready for Review.
How to test
sloppiness = pch_defines,time_macros
$ cmake . -DPRECOMPILE_HEADERS=1
$ make -j<you choose>
Measure speed:
$ cmake . -DCMAKE_CXX_COMPILER=/usr/bin/g++
$ make -B -j5