-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Remove the base.h include from source code. #2508
Comments
As far as i know, if a source file needs a subset of header files listed in pch.h, when pch.gch is found, it will be used direclty instead of precompile those original header files again, which should speed up the building speed of this translation unit. Maybe the main trouble is not the speed , namespace pollution or symbolic conflicts may happen when including many unused header files including third-party header files in pch-header.h? @Shylock-Hg |
We don't enable the precompiled header already. |
Sorry for my statement. Recently, i have implememted the cmake-pch module for my personal project and it successfully speeds up the whole compilation stage. But according to this issue, i have a little concern about the disadvantages of pch on large projects like nebula,and unsure wheteher to keep this module or not. Is there some engineering guidelines and experience in this area? @Shylock-Hg |
See the detail in #2070 |
* Fix undefined parameter Fix tck Fix tck small skip for ldbc tck fmt * small change * Fix error message * small change Co-authored-by: kyle.cao <[email protected]>
Is your feature request related to a problem? Please describe.
It's not a precompiled header again, and it include many headers which don't need in many source file. I think this will slow the compiling, so we'd better remove this include.
Describe the solution you'd like
A clear and concise description of what you want to happen.
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context or screenshots about the feature request here.
The text was updated successfully, but these errors were encountered: