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

Make build compatible of -DCMAKE_UNITY_BUILD=ON for faster builds #3962

Merged
merged 8 commits into from Nov 29, 2023

Conversation

rouault
Copy link
Member

@rouault rouault commented Nov 23, 2023

Cf https://en.wikipedia.org/wiki/Unity_build and https://cmake.org/cmake/help/latest/prop_tgt/UNITY_BUILD.html

Timings on Linux with Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz, 12 vCPUS, and "make -j12 proj"

Before:

debug, unity disabled:
real    0m37,233s
user    5m28,169s
sys     0m28,826s

release, unity disabled:
real    0m45,684s
user    6m12,384s
sys     0m25,994s

After with -DCMAKE_UNITY_BUILD=ON

debug, unity enabled:
real    0m19,870s
user    1m33,008s
sys     0m6,134s

release, unity enabled:
real    0m34,433s
user    2m20,730s
sys     0m5,395s

Debug builds with 12-threads are almost twice faster.
Release builds with 12-threads are 30% faster

And single-threaded release compilation in default mode takes 4m00s , and with CMAKE_UNITY_BUILD enabled only 1m50s

@jjimenezshaw
Copy link
Contributor

jjimenezshaw commented Nov 23, 2023

This is a "risky" feature from CMake (we suffered it in my company).
I see that compiling times are better. But I personally feel (this is a personal opinion... or feeling) that it breaks some basic "law" in C/C++, the isolation of an object file from a single cpp file. In a way that may change if you add more files or whatever.
As you already experienced, it may not compile due to collisions among anonymous namespaces. They are not "anonymous" anymore.
It may happen the other way, that something builds with unity but not without. For instance if you forget an "#include" in a cpp file, that is in a previous one.

The error messages are not easy to understand... until you remember/realize that unity may be bothering you (that happened to me at work). And that happens in CI (until you enable unity in your local machine).

In this PR only some workflows in CI are moved to unity, so we could check how it behaves with and without. I do not know if you plan to move more platforms or configurations to unity.

If we include it we should document it existence and possible consequences in Contributing or Compiling documentation.

As far as I know, the only binary/package that we release are the docker files, right? We should compile then without unity.

@rouault
Copy link
Member Author

rouault commented Nov 23, 2023

Enabling unity build will not be done by default. The end user will have to enable it explictly if he wants to. We have a mix of non-unity and unity builds in the CI to be able to catch bug specifics to both type of configuration.

If we include it we should document it existence and possible consequences in Contributing or Compiling documentation.

I've added the following note in install.rst:

.. option:: CMAKE_UNITY_BUILD=OFF

    .. versionadded:: 9.4

    Default is OFF. This can be set to ON to build PROJ using the
    https://cmake.org/cmake/help/latest/variable/CMAKE_UNITY_BUILD.html feature.
    This helps speeding PROJ build times. This feature is still considered
    experimental for now, and could hide subtle bugs (we are not aware of
    any at writing time though). We don't recommend it for mission critical
    builds.

@rouault rouault merged commit 8b22f81 into OSGeo:master Nov 29, 2023
22 checks passed
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.

None yet

2 participants