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

Support building with cmake with unity build (build speedup) #6293

Closed
bsergean opened this issue Oct 27, 2020 · 8 comments · Fixed by #6295
Closed

Support building with cmake with unity build (build speedup) #6293

bsergean opened this issue Oct 27, 2020 · 8 comments · Fixed by #6295

Comments

@bsergean
Copy link

Hi there,

With cmake > 3.16 there is a nice (optional) feature called unity build which help speedup building large or complex C++ projects. Here is a write up on how this works. Speed ups can be pretty large.

However tweaks are sometimes required to get it to work. Typically if you have 2 constants with the same value in 2 .cpp file, a problem happens. Variables with similar name needs to only be declared once.

Here is how I tried to build xgboost but I got those errors which I have a hard time decipher.

$ mkdir build ; cd build
build$ cmake -DCMAKE_UNITY_BUILD=ON -DUSE_OPENMP=OFF -DBUILD_STATIC_LIB=ON .. 
build$ 
build$ make
Scanning dependencies of target dmlc
[  7%] Building CXX object dmlc-core/CMakeFiles/dmlc.dir/Unity/unity_1_cxx.cxx.o
[ 15%] Building CXX object dmlc-core/CMakeFiles/dmlc.dir/Unity/unity_0_cxx.cxx.o
[ 23%] Linking CXX static library libdmlc.a
[ 23%] Built target dmlc
Scanning dependencies of target objxgboost
[ 30%] Building CXX object src/CMakeFiles/objxgboost.dir/Unity/unity_6_cxx.cxx.o
[ 38%] Building CXX object src/CMakeFiles/objxgboost.dir/Unity/unity_5_cxx.cxx.o
In file included from /Users/bsergeant/src/foss/github/xgboost/build/src/CMakeFiles/objxgboost.dir/Unity/unity_5_cxx.cxx:4:
/Users/bsergeant/src/foss/github/xgboost/src/predictor/predictor.cc:12:1: error: explicit specialization of 'Get' after instantiation
DMLC_REGISTRY_ENABLE(::xgboost::PredictorReg);
^
/Users/bsergeant/src/foss/github/xgboost/dmlc-core/include/dmlc/registry.h:236:47: note: expanded from macro 'DMLC_REGISTRY_ENABLE'
  Registry<EntryType > *Registry<EntryType >::Get() {                   \
                                              ^
/Users/bsergeant/src/foss/github/xgboost/src/predictor/cpu_predictor.cc:507:1: note: implicit instantiation first required here
XGBOOST_REGISTER_PREDICTOR(CPUPredictor, "cpu_predictor")
^
/Users/bsergeant/src/foss/github/xgboost/include/xgboost/predictor.h:240:54: note: expanded from macro 'XGBOOST_REGISTER_PREDICTOR'
          ::dmlc::Registry<::xgboost::PredictorReg>::Get()->__REGISTER__(Name)
                                                     ^
1 error generated.
make[2]: *** [src/CMakeFiles/objxgboost.dir/Unity/unity_5_cxx.cxx.o] Error 1
make[1]: *** [src/CMakeFiles/objxgboost.dir/all] Error 2
make: *** [all] Error 2

If anyone is interested or can give us clues on how to fix that it would be appreciated. Thanks !

@trivialfis
Copy link
Member

To fix it the factory registry in dmlc core needs to be changed.

@trivialfis
Copy link
Member

Or setup SKIP_UNITY_BUILD_INCLUSION for needed source files.

@trivialfis
Copy link
Member

Managed to get it working and tried it out. There's some speedup but in general quite similar as XGBoost is not difficult to compile, and CMake unity build doesn't support CUDA.

@bsergean
Copy link
Author

bsergean commented Oct 27, 2020 via email

@trivialfis
Copy link
Member

trivialfis commented Oct 28, 2020

@bsergean Thank you for bringing it up, it actually helps us on Windows CI build time. :-)

@bsergean
Copy link
Author

bsergean commented Oct 28, 2020 via email

@trivialfis
Copy link
Member

We are preparing 1.3 release. But will also keep you posted on merging the PR.

@trivialfis
Copy link
Member

Merged. Feel free to try it out on master.

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

Successfully merging a pull request may close this issue.

2 participants