Skip to content

Support the JNI build with nvCOMP 5.0#19309

Closed
firestarman wants to merge 5 commits intorapidsai:branch-25.10from
firestarman:build-nvcomp5
Closed

Support the JNI build with nvCOMP 5.0#19309
firestarman wants to merge 5 commits intorapidsai:branch-25.10from
firestarman:build-nvcomp5

Conversation

@firestarman
Copy link
Contributor

@firestarman firestarman commented Jul 8, 2025

Description

This PR changes the nvCOMP relevant code in JNI to match the latest API updates in nvCOMP 5.0.
cuDF cpp has done this update in #19221.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Signed-off-by: Firestarman <firestarmanllc@gmail.com>
Signed-off-by: Firestarman <firestarmanllc@gmail.com>
@firestarman firestarman requested a review from a team as a code owner July 8, 2025 13:41
@copy-pr-bot
Copy link

copy-pr-bot bot commented Jul 8, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions bot added the Java Affects Java cuDF API. label Jul 8, 2025
@firestarman firestarman requested a review from GaryShen2008 July 8, 2025 13:41
@firestarman firestarman added bug Something isn't working Spark Functionality that helps Spark RAPIDS non-breaking Non-breaking change labels Jul 8, 2025
@GaryShen2008
Copy link
Contributor

Let me run some test with nvcomp5 first.

Signed-off-by: Firestarman <firestarmanllc@gmail.com>
@firestarman
Copy link
Contributor Author

firestarman commented Jul 21, 2025

depends on #19221 and nvCOMP5.0 library.

@firestarman
Copy link
Contributor Author

/ok to test

@copy-pr-bot
Copy link

copy-pr-bot bot commented Jul 21, 2025

/ok to test

@firestarman, there was an error processing your request: E1

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/

@firestarman
Copy link
Contributor Author

/ok to test 951010e

@ttnghia ttnghia removed their assignment Jul 21, 2025
@ttnghia
Copy link
Contributor

ttnghia commented Jul 21, 2025

As discussing internally, I'm suggesting to use libcudf's internal nvcomp_adapter APIs instead of implementing our adapter like this. Doing so, we can avoid fixing the adapter every time nvcomp interface changes. In addition, libcudf also supports switching between nvcomp version while the current JNI code doesn't.

@firestarman firestarman changed the base branch from branch-25.08 to branch-25.10 August 18, 2025 03:25
@firestarman
Copy link
Contributor Author

As discussing internally, I'm suggesting to use libcudf's internal nvcomp_adapter APIs instead of implementing our adapter like this. Doing so, we can avoid fixing the adapter every time nvcomp interface changes. In addition, libcudf also supports switching between nvcomp version while the current JNI code doesn't.

This is a good idea, but it depends on the export of nvcomp_adapter APIs from the cudf cpp. We could make it as a follow-up.

@pxLi
Copy link
Member

pxLi commented Aug 18, 2025

/ok to test 0cd906a

@vuule vuule self-requested a review August 18, 2025 20:41
Copy link
Contributor

@abellina abellina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@abellina
Copy link
Contributor

It looks like we have some build failures @firestarman

@vuule
Copy link
Contributor

vuule commented Aug 19, 2025

It looks like we have some build failures @firestarman

The CI will fail on this PR until we upgrade to nvcomp 5. This is why I merged this PR with #19636. When we put the build and JNI changes together, Java tests pass 🎉

Any concerns with merging this as part of #19636?

@abellina
Copy link
Contributor

It looks like we have some build failures @firestarman

The CI will fail on this PR until we upgrade to nvcomp 5. This is why I merged this PR with #19636. When we put the build and JNI changes together, Java tests pass 🎉

Any concerns with merging this as part of #19636?

That sounds great, thanks @vuule

@vuule
Copy link
Contributor

vuule commented Aug 19, 2025

Any concerns with merging this as part of #19636?

That sounds great, thanks @vuule

In that case, feel free to transfer your reviews over to #19636 :)

@vuule vuule mentioned this pull request Aug 19, 2025
3 tasks
@vyasr
Copy link
Contributor

vyasr commented Aug 20, 2025

Can we close this now that #19636 is merged?

@ttnghia
Copy link
Contributor

ttnghia commented Aug 20, 2025

Can we close this now that #19636 is merged?

Close.

@ttnghia ttnghia closed this Aug 20, 2025
@firestarman firestarman deleted the build-nvcomp5 branch August 22, 2025 08:11
@firestarman
Copy link
Contributor Author

thx all

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

Labels

bug Something isn't working Java Affects Java cuDF API. non-breaking Non-breaking change Spark Functionality that helps Spark RAPIDS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

Comments