Skip to content

Add support for nvcomp-2.3 in java build - #10837

Closed
jbrennan333 wants to merge 5 commits into
NVIDIA:branch-22.06from
jbrennan333:jtb-nvcomp-2.3
Closed

jbrennan333 wants to merge 5 commits into
NVIDIA:branch-22.06from
jbrennan333:jtb-nvcomp-2.3

Conversation

@jbrennan333

Copy link
Copy Markdown
Contributor

Closes #10681.

Add support to the Java build to use nvcomp-2.3 instead of nvcomp-2.2. The main difference is that nvcomp-2.2 was included as a static library built by CUDF CPP while nvcomp-2.3 is only available as shared libraries that must be downloaded separately.

I'm putting this up as a draft pr so I can get feedback, and also because merging this without additional changes to pull in nvcomp-2.3 automatically in CUDF are needed. If we merge this without those changes, the java build will succeed, but java unit tests will fail and the resulting jars will not work. This is primarily because of the changes to the NativeDepsLoader, which will fail if it does not find the nvcomp-2.3 shared libs.

Note that @robertmaynard is working on rapids_cpm_nvcomp. Once that is available, we should be able to update the changes here and combine them with the changes needed in CUDF CPP so we can do everything in one PR.

The spark-rapids-jni project will also require a corresponding change. I will file PR in that project as well.

Another reason to hold off on making this change is that nvcomp-2.3 shared libs are not compiled with CUDA_API_PER_THREAD_DEFAULT_STREAM. This is a performance issue for spark-rapids - see previous issue: #9534

@jbrennan333 jbrennan333 added CMake CMake build issue Spark Functionality that helps Spark RAPIDS DO NOT MERGE Hold off on merging; see PR for details improvement Improvement / enhancement to an existing function breaking Breaking change not a bug labels May 12, 2022
@jbrennan333 jbrennan333 self-assigned this May 12, 2022
@github-actions github-actions Bot added the Java Affects Java cuDF API. label May 12, 2022
@jbrennan333

Copy link
Copy Markdown
Contributor Author

I tried using cmake install to do the copies with code like this:

if(TARGET nvcomp::nvcomp)
  install(
    IMPORTED_RUNTIME_ARTIFACTS nvcomp::nvcomp nvcomp::nvcomp_gdeflate nvcomp::nvcomp_bitcomp
    LIBRARY DESTINATION "${PROJECT_BINARY_DIR}"
  )
endif()

But IMPORTED_RUNTIME_ARTIFACTS requires cmake 3.21 or greater, and the spark-rapids-jni docker image currently has cmake version 3.20..5. So for now I went back to the copy method.

cc: @robertmaynard

Comment thread java/src/main/native/CMakeLists.txt Outdated
Comment thread java/src/main/java/ai/rapids/cudf/NativeDepsLoader.java
@robertmaynard

Copy link
Copy Markdown
Contributor

I have opened a PR to add rapids_cpm_nvcomp rapidsai/rapids-cmake#190

@jbrennan333

Copy link
Copy Markdown
Contributor Author

I don't understand what that checkstyle issue is?

@jlowe

jlowe commented May 16, 2022

Copy link
Copy Markdown
Contributor

I don't understand what that checkstyle issue is?

It's CMake formatting:

cmake-format.............................................................Failed
- hook id: cmake-format
- files were modified by this hook

You probably need to run cpp/scripts/run-cmake-format.sh cmake-format java/src/main/native/CMakeLists.txt.

@jbrennan333

Copy link
Copy Markdown
Contributor Author

These changes were pulled into #10851, so this draft PR can be closed.

rapids-bot Bot pushed a commit that referenced this pull request May 25, 2022
When building cudf on x86_64 hardware we can enable the new pre-built nvcomp 2.3 binaries to leverage new compression codec support.

Closes #10681
Closes #10837

Authors:
  - Robert Maynard (https://github.com/robertmaynard)
  - Jim Brennan (https://github.com/jbrennan333)
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Jason Lowe (https://github.com/jlowe)
  - AJ Schmidt (https://github.com/ajschmidt8)
  - Jim Brennan (https://github.com/jbrennan333)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #10851
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking change CMake CMake build issue DO NOT MERGE Hold off on merging; see PR for details improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. Spark Functionality that helps Spark RAPIDS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEA] Update Java bindings to handle separate nvcomp library

3 participants