-
-
Notifications
You must be signed in to change notification settings - Fork 839
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
WIP: Adds precompiled headers for MSVC to speed up compiling. #200
Conversation
I am not sure I really like that. While I agree that the compile time is too long, I think the proper solution would be to avoid dependencies between the codes by moving some of the algos into separate files (even eventually in C for extra compilation speed). This patch also further separates the linux build from the Windows build, which is not a good thing in my opinion. |
Are precompiled headers a compilation speedup or do they something that changes the end result? On Linux cmake detects ccache to speedup compilation, so I would not oppose if this does not break other things. Maybe make it a cmake option? |
@gzotti PCH doesn't change the end result, it just speed up compiling by pre compile common used headers. If those headers are included by different .cpp, they'll be compiled over and over again. Instead, if they are put into a PCH, they get compiled only once, and then reference that compiled binary. @guillaumechereau Understand your concern. Yes, it's creating a separation between msvc and other compilers. However, using PCHs in msvc and gcc/clang have different compiler options, it has to be different. Avoid dependencies is not conflict with PCH, it can be benefited by PCH as well. |
Why not to use https://github.com/sakra/cotire? It's a crossplatform precompiled headers generator for CMake. |
That looks interesting! |
I'll try cotire. Thanks @375gnu . |
Just tried cotire, run into this problem... |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
According to https://github.com/sakra/cotire cotire has been superseded by functionality built into cmake 3.16. Any volunteers to bring this forward? (Note that this must remain optional as long as we don't ask for cmake 3.16+) |
@gongminmin have you tried the cmake way to PCH? Else IMHO we should close this PR. |
@gongminmin any news? |
I close this. OP is unresponsive, and CMake options can be developed in a new PR. |
Appveyor building time drops from 9.5 mins to 7.5 mins. On my PC, the time of building stelMain reduces from 1 min to 35 secs.
I also have some code to enable PCH in GCC/Clang. Unfortunately, it has some problem with including Qt headers. So I didn't put that code into this pull request.