Use rtcx from the librtcx repository by vyasr · Pull Request #23137 · NVIDIA/cudf

vyasr · 2026-07-07T05:48:07Z

Description

This PR removes librtcx code from libcudf and repoints it to use the new librtcx repository.

Checklist

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

coderabbitai · 2026-07-07T05:49:45Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR switches cuDF to fetch rtcx via CPM, updates CMake wiring to use the fetched dependency layout, and removes the in-tree rtcx implementation body.

Changes

librtcx CPM migration

Layer / File(s)	Summary
New get_rtcx.cmake CPM fetch script `cpp/cmake/thirdparty/get_rtcx.cmake`	Defines `find_and_configure_rtcx(VERSION)` to fetch rtcx with `rapids_cpm_find`, conditionally include `embed.cmake`, and invoke the function with `RTCX_MIN_VERSION_cudf`.
Wire CMakeLists.txt to fetched rtcx source `cpp/CMakeLists.txt`	Replaces the local `librtcx` subdirectory hookup with the fetch script, updates `cudf_cuda_embed` to read `libcxx` from `${rtcx_SOURCE_DIR}`, and removes the local `librtcx` include path from `cudf`.
Remove in-tree rtcx implementation `cpp/librtcx/rtcx.cpp`	Removes the body of the in-tree rtcx runtime implementation, including cache, compile/link, and decompression logic.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

rapidsai/cudf#21625: Directly overlaps with replacing in-tree rtcx embed/build plumbing.
rapidsai/cudf#22654: Also changes rtcx/embed CMake integration in cpp/CMakeLists.txt.
rapidsai/cudf#22744: Touches RTCX build/runtime integration and cpp/librtcx implementation files.

Suggested reviewers: lamarrr, ttnghia, bdice

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: switching libcudf to use rtcx from the external librtcx repository.
Description check	✅ Passed	The description directly matches the changeset by explaining that librtcx code is removed from libcudf and replaced with the external repository.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/cmake/thirdparty/get_rtcx.cmake`:
- Around line 13-14: The rtcx dependency is still pinned to a personal fork in
the get_rtcx.cmake fetch logic. Update the GIT_REPOSITORY used by the rtcx
download step to the official rapidsai/librtcx source and replace the raw SHA
pin with the appropriate maintained tag or release reference, keeping the change
localized to the get_rtcx.cmake configuration.
- Around line 19-23: The rtcx setup only includes embed.cmake when rtcx is added
from source, so the embed helper functions are missing when rtcx is found via
find_package. Update get_rtcx.cmake so embed.cmake is included for the
installed-package path as well, using the rtcx_SOURCE_DIR/rtcx_DIR-related
symbols in that logic, and ensure the rtcx_add_embed and rtcx_embed_* functions
are available regardless of whether rtcx comes from CPM or an installed package.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6656af1d-edec-411b-a92e-79ecf61f1141

📥 Commits

Reviewing files that changed from the base of the PR and between ba255bb and 6c57281.

📒 Files selected for processing (21)

cpp/CMakeLists.txt
cpp/cmake/thirdparty/get_rtcx.cmake
cpp/librtcx/CMakeLists.txt
cpp/librtcx/CONTRIBUTING.md
cpp/librtcx/LICENSE
cpp/librtcx/RAPIDS_BRANCH
cpp/librtcx/README.md
cpp/librtcx/VERSION
cpp/librtcx/cmake/RAPIDS.cmake
cpp/librtcx/cmake/rapids_config.cmake
cpp/librtcx/embed.cmake
cpp/librtcx/embed.hpp
cpp/librtcx/embed.in.cpp
cpp/librtcx/hash.cpp
cpp/librtcx/hash.hpp
cpp/librtcx/libcxx/cassert
cpp/librtcx/libcxx/climits
cpp/librtcx/libcxx/cstddef
cpp/librtcx/libcxx/cstdint
cpp/librtcx/rtcx.cpp
cpp/librtcx/rtcx.hpp

💤 Files with no reviewable changes (19)

cpp/librtcx/RAPIDS_BRANCH
cpp/librtcx/CONTRIBUTING.md
cpp/librtcx/libcxx/cassert
cpp/librtcx/libcxx/cstddef
cpp/librtcx/libcxx/cstdint
cpp/librtcx/hash.hpp
cpp/librtcx/cmake/RAPIDS.cmake
cpp/librtcx/embed.in.cpp
cpp/librtcx/VERSION
cpp/librtcx/README.md
cpp/librtcx/embed.cmake
cpp/librtcx/LICENSE
cpp/librtcx/CMakeLists.txt
cpp/librtcx/hash.cpp
cpp/librtcx/libcxx/climits
cpp/librtcx/embed.hpp
cpp/librtcx/cmake/rapids_config.cmake
cpp/librtcx/rtcx.hpp
cpp/librtcx/rtcx.cpp

rtcx is statically linked into libcudf and should not appear in the installed CMake export set. Wrap it with BUILD_LOCAL_INTERFACE (matching the xxhash pattern) so downstream consumers do not need to find rtcx separately. Also remove the stale include path to the now-deleted bundled cpp/librtcx/ directory; headers are provided by the rtcx::rtcx target.

Three changes: 1. Set RTCX_INSTALL=ON before fetching rtcx so that librtcx's install targets are unconditionally registered (no EXCLUDE_FROM_ALL). This satisfies CMake's install(EXPORT) validation which requires all link dependencies to be in some export set. 2. Add rapids_export_find_package_root(BUILD rtcx ...) so the build-tree export set can locate the rtcx package. 3. Add rtcx to the static-deps export list so that static libcudf consumers (e.g. spark-rapids-jni) get find_dependency(rtcx) in the installed cudf config and can link librtcx.a. 4. Revert BUILD_LOCAL_INTERFACE on rtcx::rtcx since the target must remain visible to the install export set.

The new commit defers zstd/xxhash target checks from embed.cmake include time to rtcx_embed() call time, so consumers that only link librtcx (like spark-rapids-jni) don't need these targets available.

vyasr · 2026-07-07T23:30:07Z

There are going to be a couple of major changes in the librtcx repository in quick succession after it's created, and we want to pin to a commit rather than a tag for now while we're still at such a raw alpha stage, so I'm going to use this PR to step through the first few quick changes before we merge and actually migrate away from the in-source librtcx.

vyasr · 2026-07-08T00:36:45Z

https://github.com/rapidsai/cudf/actions/runs/28900128150?pr=23137 is the passing run verifying rapidsai/librtcx#2.

vyasr · 2026-07-08T20:23:30Z

https://github.com/rapidsai/cudf/actions/runs/28960562453?pr=23137 is the passing test verifying rapidsai/librtcx#3

The cuVS upstreaming (rapidsai/librtcx#8) added a raft dependency that breaks the cmake-tests export consumer. Pin to efad266 which is the last commit before that addition.

The previous commit used an incorrect hash. Use the actual full SHA for the 'Add and apply pre-commit hooks (#6)' commit.

vuule

Second C++ review :D

vyasr · 2026-07-13T15:13:04Z

/merge

vyasr self-assigned this Jul 7, 2026

vyasr requested review from a team as code owners July 7, 2026 05:48

vyasr added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jul 7, 2026

vyasr requested review from mattgara and vuule July 7, 2026 05:48

github-actions Bot added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue labels Jul 7, 2026

vyasr added 3 commits July 6, 2026 22:48

feat: fetch librtcx via CPM instead of add_subdirectory

0764ae6

chore: remove bundled librtcx sources

185d02f

ci: temporarily point rtcx CPM fetch at vyasr/librtcx@31a7295

6c57281

vyasr force-pushed the feat/external_rtccx branch from 2ad9d91 to 6c57281 Compare July 7, 2026 05:48

coderabbitai Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread cpp/cmake/thirdparty/get_rtcx.cmake Outdated

Comment thread cpp/cmake/thirdparty/get_rtcx.cmake

bdice approved these changes Jul 7, 2026

View reviewed changes

Comment thread cpp/cmake/thirdparty/get_rtcx.cmake Outdated

bdice approved these changes Jul 7, 2026

View reviewed changes

vyasr force-pushed the feat/external_rtccx branch from 2bd8069 to 2720c1b Compare July 7, 2026 21:05

ci: update rtcx pin to include embed.cmake lazy-check fix

f797da4

The new commit defers zstd/xxhash target checks from embed.cmake include time to rtcx_embed() call time, so consumers that only link librtcx (like spark-rapids-jni) don't need these targets available.

vyasr added the DO NOT MERGE Hold off on merging; see PR for details label Jul 7, 2026

ci: update rtcx pin to split-embed-cmake branch head

cb37005

vyasr added 2 commits July 10, 2026 10:24

Update to latest commit

4d97501

Merge remote-tracking branch 'upstream/main' into feat/external_rtccx

1d4a13f

vyasr removed the DO NOT MERGE Hold off on merging; see PR for details label Jul 10, 2026

Fix org

bf43e3a

vyasr added 2 commits July 10, 2026 14:17

Pin librtcx to pre-cuVS commit

2dec5d8

The cuVS upstreaming (rapidsai/librtcx#8) added a raft dependency that breaks the cmake-tests export consumer. Pin to efad266 which is the last commit before that addition.

Fix librtcx commit hash

7849e42

The previous commit used an incorrect hash. Use the actual full SHA for the 'Add and apply pre-commit hooks (#6)' commit.

vuule approved these changes Jul 10, 2026

View reviewed changes

Merge branch 'main' into feat/external_rtccx

9f20542

rapids-bot Bot merged commit 4526245 into NVIDIA:main Jul 13, 2026
136 checks passed

vyasr deleted the feat/external_rtccx branch July 13, 2026 16:47

coderabbitai Bot mentioned this pull request Jul 22, 2026

[FEA] Multi-String Output & LTO Transform Examples #23173

Open

3 tasks

Navigation Menu

Uh oh!

Use rtcx from the librtcx repository - #23137

Use rtcx from the librtcx repository#23137
rapids-bot[bot] merged 13 commits into
NVIDIA:mainfrom
vyasr:feat/external_rtccx

vyasr commented Jul 7, 2026

Uh oh!

coderabbitai Bot commented Jul 7, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vyasr commented Jul 7, 2026

Uh oh!

vyasr commented Jul 8, 2026

Uh oh!

vyasr commented Jul 8, 2026

Uh oh!

vuule left a comment

Uh oh!

vyasr commented Jul 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

vyasr commented Jul 7, 2026

Description

Checklist

Uh oh!

coderabbitai Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vyasr commented Jul 7, 2026

Uh oh!

vyasr commented Jul 8, 2026

Uh oh!

vyasr commented Jul 8, 2026

Uh oh!

vuule left a comment

Choose a reason for hiding this comment

Uh oh!

vyasr commented Jul 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Jul 7, 2026 •

edited

Loading