Add unity build support for some targets #9125
Conversation
|
@spoonincode do you see any issues with providing this option? |
Up until cmake version 3.17.*, The unity build support by cmake does not work well in combination with the production of compile_commands.json. This commit allows the short-term workaround: running cmake twice: once with ENABLE_UNITY_BUILD=OFF to generate compile_commands.json for LSP servers, clang-tidy / clangd, etc, and then again with ENABLE_UNITY_BUILD=ON to build the project.
|
I played with it some and I guess not. It does feel a little hokey needing to sprinkle these all through various cmakefiles. Unless someone is actively using the feature it risks becoming wrong/broken like a handful of other cmake stuff we have sprinkled about still. |
IMO, the option should be enabled by default for CI build. This is build times I got from comparing the CI build with the ENABLE_UNITY_BUILD ON and OFF.
Personally, I don't use build script in the EOS repo. I prefer to supply the cmake options I need and use ninja. Without unity build, using "ninja" for clean build without limiting jobs would freeze my Mac with 32 GB ram, I need to explicit limit the jobs to 4 to avoid it. With unity build, I don't need to specify the number of jobs and it also compiles a lot faster. I can even go as far as 18 jobs on my 8-core machine without any issues. |
| @@ -118,7 +118,7 @@ namespace fc { namespace crypto { | |||
| if (which == 0) { | |||
| using default_type = storage_type::template type_at<0>; | |||
| return to_wif(_storage.template get<default_type>(), yield); | |||
| } | |||
| } | |||
There was a problem hiding this comment.
stray space at end of line
|
Should we be keeping CI & build script in parity? |
Change Description
This PR add support to enable cmake unity build for certain targets. Enabling unity build can significantly improve the build time for the project. However, it's not possible enable it globally with CMAKE_UNITY_BUILD because some of the libraries/submodules requires modification to make it work. This PR uses ENABLE_UNITY_BUILD flag to enable unity build for the targets don't have problem with it.
Consensus Changes
API Changes
Documentation Additions