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

Add unity build #131

Open
wants to merge 1 commit into
base: oxce-plus
Choose a base branch
from
Open

Conversation

yarolig
Copy link

@yarolig yarolig commented Mar 18, 2024

TLDR

Apply my changes and run this:

  mkdir build.d; cd build.d
  cmake .. -G Ninja -DCMAKE_UNITY_BUILD=true -DCMAKE_BUILD_TYPE=Release -DDEV_BUILD=ON -DBUILD_PACKAGE=OFF
  time /usr/bin/time -v ninja

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

#include "/path/to/ArrowButton.cpp"
#include "/path/to/Bar.cpp"
#include "/path/to/BattlescapeButton.cpp"
...

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\ )

  • The build failed with some linking errors.
  • Unity build seems to be working... partially... I get compilation errors that I do not understand
  • Visual Studio has its own "Unity (JUMBO) build". You can turn it on in project properties. It does not respect SKIP_UNITY_BUILD_INCLUSION so it fails when it sees conflicting symbols.

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 with add_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 of add_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.

@Yankes
Copy link
Collaborator

Yankes commented Mar 18, 2024

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.

How exactly this PR help with this? Overall most of "cost" of changes like this lie in breaking some builds setups.
If you want grain some experience and have chance to have this PR accepted is to first check if all main builds do not break.
You can ask on openxcom forums for people who regularly builds some less available builds (like Mac) for feedback and if your change do not break any thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants