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

Rework JNI CMake to leverage rapids_find_package #10649

Merged
merged 6 commits into from
Apr 18, 2022

Conversation

jlowe
Copy link
Member

@jlowe jlowe commented Apr 13, 2022

The JNI CMakeLists.txt has been fragile, looking for specific .a or .so libraries and header file locations, and will break again when libcudf moves to a pre-installed nvcomp 2.3 package since it expects to find nvcomp in a very specific location today. This refactors the JNI CMakeLists.txt to leverage rapids_find_package to reuse the work performed in the libcudf build and also has the nice side-effect of avoiding redundant pulls and builds of the Thrust and RMM repositories that is happening today.

Another side-effect is that the JNI will now automatically pull in the same RMM compile definitions that are used for libcudf, meaning the separate RMM logging flag for the JNI build is no longer necessary. Similarly it's no longer necessary to explicitly specify to the JNI build which type of Arrow library to use (i.e,.: static or dynamic), it will automatically use whichever Arrow library was built by libcudf.

@jlowe jlowe added CMake CMake build issue Java Affects Java cuDF API. 4 - Needs cuDF (Java) Reviewer improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 13, 2022
@jlowe jlowe requested a review from a team as a code owner April 13, 2022 14:53
@jlowe jlowe self-assigned this Apr 13, 2022
@codecov
Copy link

codecov bot commented Apr 13, 2022

Codecov Report

Merging #10649 (b1748f6) into branch-22.06 (8f5a044) will increase coverage by 0.06%.
The diff coverage is 57.14%.

❗ Current head b1748f6 differs from pull request most recent head 86c2555. Consider uploading reports for the commit 86c2555 to get more accurate results

@@               Coverage Diff                @@
##           branch-22.06   #10649      +/-   ##
================================================
+ Coverage         86.34%   86.41%   +0.06%     
================================================
  Files               142      142              
  Lines             22356    22334      -22     
================================================
- Hits              19304    19300       -4     
+ Misses             3052     3034      -18     
Impacted Files Coverage Δ
python/cudf/cudf/core/dataframe.py 93.75% <ø> (-0.01%) ⬇️
python/cudf/cudf/utils/cudautils.py 65.74% <ø> (+5.90%) ⬆️
python/cudf/cudf/core/series.py 95.15% <57.14%> (-0.13%) ⬇️
python/cudf/cudf/core/groupby/groupby.py 91.72% <0.00%> (+0.22%) ⬆️
python/cudf/cudf/core/column/string.py 89.22% <0.00%> (+0.24%) ⬆️
python/cudf/cudf/core/tools/datetimes.py 84.49% <0.00%> (+0.30%) ⬆️
python/cudf/cudf/core/column/lists.py 92.79% <0.00%> (+1.27%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e668f2...86c2555. Read the comment docs.

@jlowe
Copy link
Member Author

jlowe commented Apr 15, 2022

Note that the previous CI failures were due to problems with the cuco dependency in a shared library build setup which should be addressed by #10662.

@jlowe
Copy link
Member Author

jlowe commented Apr 18, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 9409559 into rapidsai:branch-22.06 Apr 18, 2022
@jlowe jlowe deleted the jni-cmake-rework branch April 18, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake CMake build issue improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants