Skip to content
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

Use precompiled headers to improve compile times. #43

Merged
merged 1 commit into from
Jan 5, 2022

Conversation

petebannister
Copy link

Drastic improvement to compile time with MSVC2019. You may want to adjust the way that USE_PRECOMPILED_HEADERS gets set to restrict the compilers it applies to. I have only tried this with MSVC. I believe GCC implements PCH these days. Not sure r.e. clang -- I think it does a good job without them.

@petebannister
Copy link
Author

On my system, PCH means full built takes around 8 minutes, vs around 50 minutes for non-pch build.

@petebannister
Copy link
Author

Mentioned this in issue #5 also

@gengjiawen
Copy link
Collaborator

Wow, amazing work.

@gengjiawen
Copy link
Collaborator

Any chance we can also try this on gcc in CI ?

@gengjiawen
Copy link
Collaborator

@petebannister Do you have any interest bring this to node.js ? Currently we have pch, but not this kind of fast. https://github.com/nodejs/node/blob/master/tools/msvs/pch/v8_pch.h

@petebannister
Copy link
Author

@petebannister Do you have any interest bring this to node.js ? Currently we have pch, but not this kind of fast. https://github.com/nodejs/node/blob/master/tools/msvs/pch/v8_pch.h

Node itself is not something I've built before... perhaps it should use v8-cmake instead? ;) It is so much easier to build (and debug the build) than the GN build and also takes up way less disk space.

I'm using this to embed v8 into an application (as a conan package).

A big saving was a specific PCH for the torque generated code (which can't apply to all libraries). There are some weird effects when including some headers in the PCH that break the build. Some files don't have inclusion guards or there are definition ordering issues. In some cases there are some globbed headers but its risky due to potential include ordering breakage.

@gengjiawen
Copy link
Collaborator

@petebannister Do you have any interest bring this to node.js ? Currently we have pch, but not this kind of fast. https://github.com/nodejs/node/blob/master/tools/msvs/pch/v8_pch.h

Node itself is not something I've built before... perhaps it should use v8-cmake instead? ;) It is so much easier to build (and debug the build) than the GN build and also takes up way less disk space.

I'm using this to embed v8 into an application (as a conan package).

A big saving was a specific PCH for the torque generated code (which can't apply to all libraries). There are some weird effects when including some headers in the PCH that break the build. Some files don't have inclusion guards or there are definition ordering issues. In some cases there are some globbed headers but its risky due to potential include ordering breakage.

Thanks for the explaination. @bnoordhuis What do you think of this ?

Also, master is v8 8.8 now :)

@gengjiawen
Copy link
Collaborator

I tried on gcc and clang, also works, thx.

@gengjiawen gengjiawen merged commit 1518417 into bnoordhuis:8.7 Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants