Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TLDR
Apply my changes and run this:
and OpenXcom will compile 2 times faster!
What is unity build
Usually C++ project consists of nice and small .cpp files that uses big and template heavy C++ standard library.
Compilling such project is inefficient because you need to reprocess lots of header files each time.
Unity build tries to reduce the time wasted on recompiling headers.
CMake prepares a unity_N.cxx files that looks like
And compiles these files instead.
The docs for cmake unity build are here https://cmake.org/cmake/help/latest/prop_tgt/UNITY_BUILD.html
There are independent implementations of this technique in CMake, meson, Visual Studio and other build systems. This PR is for CMake.
The good parts
Not only it may reduce compile time. It allows to do more optimisations and may speed up linking.
On my system the compilation time reduced from 3 minutes to 1m30s.
I hope android developers will be happy if it gets ported...
Drawbacks
The unity build can cause compillation errors and even runtime issues!
The source code should be "good enough" to allow unity builds.
The same names in different .cpp files (even in anonymous namespaces or static), abuse of #defines/#includes and other unusual practices are incompatible.
Many of 3rd party libraries are incompatible.
The unity build may interfere with ccache and precompiled headers. But building from scratch thould still be faster.
It also may limit the effect of using multiple CPU cores and increase memory requirements.
By default the unity build group files by 8 (and it works really well for OpenXcom, 16 is also good, other values are not so effective).
It works with cmake >=3.18 (older versions just ignore this settings).
So if you want to use it on Android... It is not trivial.
I do not understand gradle and I tried to compile for Android... There is a separate CMakeLists.txt. It builds with gradle 4.10 and cmake 3.6. I get errors when I try to replace cmake (meters of stack traces, something about parsing versions or just NullPointerException). Also upgrades require killing gradle server and removing .gradle directory. And gradle scripts need some changes when upgrading the gradle version. The execute() method is removed (https://docs.gradle.org/current/userguide/upgrading_version_4.html) and I do not know how to replace it. I guess it better to move the code for zips to cmake.I see there is a pull request with updates but have not checked it MeridianOXC/openxcom-android#3
I have not checked how it works with MSVC or Apple.I checked mxe build on Ubuntu 22.04, it is ok, unity build works as expected.
On Windows everything seems to be ok if unity build is turned off.
I tried to build openxcom with fresh Visual Studio and it is... not trivial...
I can compile vs2010 solution (using this steps from https://openxcom.org/forum/index.php?topic=7048.0).
I'm not sure how my changes in CMakeLists.txt are propagated to VS project.
My version of CMake does not generate vs2010 projects. I tried vs2022.
I got a project with no /MP option (same as -j in make). Need to turn it on manually.
Not all build tools for Windows 10 understand C++ that used in OpenXcom. I need to manually change build tool version in project properties.
I need to remove this line because MS C++ compiler do not understand this flag:
set_property ( SOURCE Engine/Scalers/xbrz.cpp APPEND_STRING PROPERTY COMPILE_FLAGS -Wno-unused\ )
I asked for help on oxce forums (https://openxcom.org/forum/index.php?topic=11916.0)
How to convert a project to unity build
Run cmake with
-DCMAKE_UNITY_BUILD=true
Source files should have empty COMPILE_FLAGS property, see https://devdocs.io/cmake~3.26/prop_tgt/unity_build
Incompatible files may be excluded by setting SKIP_UNITY_BUILD_INCLUSION property to true.
To make source files compatible the conflicting symbols should be renamed or be placed in unique namespaces.
My changes
I need to remove COMPILE_FLAGS property so I replaced
set_property
withadd_compile_options
.I compared new and old ninja.build files. The difference are in whitespace and in compiler flag -Wno-unused (flag order changed for xbrz.cpp).
So I go on and enabled the unity build. I excluded libraries and some game files.
I have not changed any .cpp files yet.
The CMake docs recommends to leave unity build off by default and allow the user to decide so it is off by default.
I'm not sure about...
It is unclear why there is
set_property
instead ofadd_compile_options
. It was introduced in a9b4670#diff-148715d6ea0c0ea0a346af3f6bd610d010d490eca35ac6a9b408748f7ca9e3f4R414 is it required on MacOS?I have not compiled the code on Windows or on Apple.Windows build unaffected but not will not benefits of unity build.
Apple build unchecked.
Why am I doing this
To be honest I want to push the unity build to much bigger project. I do not know CMake well.
So I want to try first with something smaller and simpler.