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 RABIT_DLL tag to definitions of rabit APIs. #140

Merged
merged 4 commits into from
May 19, 2020

Conversation

trivialfis
Copy link
Member

@trivialfis trivialfis commented May 15, 2020

  • Fix CI build.
  • Fix hidden symbols.

@ShvetsKS Could you try -DHIDE_CXX_SYMBOLS=ON again with this patch enabled?

Related: dmlc/xgboost#5666 (comment)

@ShvetsKS
Copy link

ShvetsKS commented May 15, 2020

Thanks for fast investigation. Unfortunately I still have this undefined symbol error on 3220a69.

I fetch your changes for rabit repo, and used docker build:
sudo tests/ci_build/ci_build.sh gpu_build docker -it --build-arg CUDA_VERSION=10.0 tests/ci_build/build_via_cmake.sh -DOPEN_MP:BOOL=ON -DHIDE_CXX_SYMBOLS=ON

@trivialfis
Copy link
Member Author

trivialfis commented May 15, 2020

@ShvetsKS I will come back to this next week. It's weird as now our pip package is by default built with hidden symbols. Also I tried the docker build last week ..

@ShvetsKS
Copy link

ShvetsKS commented May 15, 2020

some remark: if I use 1.1.0RC2/RC1 I can run my benchmark and there is no such error (undefined symbol: RabitGetRank). But if I try my custom docker build with -DHIDE_CXX_SYMBOLS=ON I get error.
It's a little bit confusing if 1.1.0RC2/RC1 was built with -DHIDE_CXX_SYMBOLS=ON

@trivialfis
Copy link
Member Author

@ShvetsKS RC2 should be built hidden symbols as it was to fix a bug where dmlc-core from xgboost clashes with dmlc-core from tvm.

Could you try running objdump -t libxgboost.so | grep "Rabit" and see what symbols are exported?

@trivialfis
Copy link
Member Author

Sorry, my bad. Rebased onto master.

@trivialfis
Copy link
Member Author

It should work now.

@trivialfis
Copy link
Member Author

Closes #137 .

@trivialfis trivialfis requested a review from hcho3 May 17, 2020 17:23
@trivialfis trivialfis merged commit a6008d5 into dmlc:master May 19, 2020
@trivialfis trivialfis deleted the add-dll-tag branch May 19, 2020 10:20
@hcho3 hcho3 mentioned this pull request Jun 5, 2020
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.

3 participants