Skip to content

Replace custom cuda_pinned_resource with RMM's pinned_host_memory_resource#1466

Merged
rapids-bot[bot] merged 4 commits intorapidsai:mainfrom
bdice:refactor-pinned-resource
Oct 30, 2025
Merged

Replace custom cuda_pinned_resource with RMM's pinned_host_memory_resource#1466
rapids-bot[bot] merged 4 commits intorapidsai:mainfrom
bdice:refactor-pinned-resource

Conversation

@bdice
Copy link
Copy Markdown
Contributor

@bdice bdice commented Oct 25, 2025

Removes custom cuda_pinned_resource implementation and uses rmm::mr::pinned_host_memory_resource directly.

Depends on rapidsai/rmm#2102.

Closes #754.

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Oct 25, 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.

@bdice bdice changed the base branch from branch-25.08 to main October 25, 2025 18:11
@github-actions github-actions Bot added the cpp label Oct 25, 2025
@bdice bdice self-assigned this Oct 25, 2025
@bdice bdice added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Oct 25, 2025
@bdice bdice moved this from Todo to In Progress in Unstructured Data Processing Oct 25, 2025
@achirkin
Copy link
Copy Markdown
Contributor

/ok to test

rapids-bot Bot pushed a commit to rapidsai/rmm that referenced this pull request Oct 27, 2025
Makes `pinned_host_memory_resource` derive from `device_memory_resource` by implementing the standard `do_allocate`, `do_deallocate`, and `do_is_equal` interface.

This is one step contributing to #2090 and is a prerequisite for #1429 / #2105.

Unblocks follow-up work:
- rapidsai/cudf#20003
- rapidsai/cuvs#1466

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #2102
@bdice
Copy link
Copy Markdown
Contributor Author

bdice commented Oct 27, 2025

@achirkin It looks like the CI run you triggered is passing. However, this should not have passed until the RMM PR was merged (which just happened).

Is this code not being tested in CI?

@bdice bdice marked this pull request as ready for review October 27, 2025 17:52
@bdice bdice requested a review from a team as a code owner October 27, 2025 17:52
@achirkin
Copy link
Copy Markdown
Contributor

Oh. Yes, this is a part of benchmarks, which we don't run in CI atm.

@bdice
Copy link
Copy Markdown
Contributor Author

bdice commented Oct 29, 2025

@achirkin Can you help build benchmarks on this PR to manually validate it? I tried -DBUILD_CUVS_BENCHMARKS=ON in a devcontainer and eventually ran out of effort that I was willing to expend after dealing with a series of build failures. If you don't have time let's just merge it.

I gave up here:

[32/527] Building CXX object _deps/diskann-build/src/CMakeFiles/diskann.dir/pq_flash_index.cpp.o
FAILED: _deps/diskann-build/src/CMakeFiles/diskann.dir/pq_flash_index.cpp.o
/usr/bin/sccache /home/coder/.conda/envs/rapids/bin/x86_64-conda-linux-gnu-c++ -DMKL_ILP64 -DRELEASE_UNUSED_TCMALLOC_MEMORY_AT_CHECKPOINTS -Ddiskann_EXPORTS -I/home/coder/cuvs/cpp/build/conda/cuda-13.0/release/_deps/diskann-src/include -I/home/coder/cuvs/cpp/build/conda/cuda-13.0/release/_deps/diskann-src/gperftools/src -isystem /home/coder/.conda/envs/rapids/include -fvisibility-inlines-hidden -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /home/coder/.conda/envs/rapids/include  -I/home/coder/.conda/envs/rapids/targets/x86_64-linux/include -I/home/coder/.conda/envs/rapids/targets/x86_64-linux/include/cccl -fopenmp -mavx2 -mfma -msse2 -ftree-vectorize -fno-builtin-malloc -fno-builtin-calloc -fno-builtin-realloc -fno-builtin-free -fopenmp -fopenmp-simd -funroll-loops -Wfatal-errors -DUSE_AVX2 -fno-finite-math-only -laio -O3 -DNDEBUG -DNDEBUG -Ofast -march=native -mtune=native -std=gnu++17 -fPIC -m64 -Wl,--no-as-needed -DMKL_ILP64 -Werror -MD -MT _deps/diskann-build/src/CMakeFiles/diskann.dir/pq_flash_index.cpp.o -MF _deps/diskann-build/src/CMakeFiles/diskann.dir/pq_flash_index.cpp.o.d -o _deps/diskann-build/src/CMakeFiles/diskann.dir/pq_flash_index.cpp.o -c /home/coder/cuvs/cpp/build/conda/cuda-13.0/release/_deps/diskann-src/src/pq_flash_index.cpp
In file included from /home/coder/.conda/envs/rapids/lib/gcc/x86_64-conda-linux-gnu/14.3.0/include/x86gprintrin.h:105,
                 from /home/coder/.conda/envs/rapids/lib/gcc/x86_64-conda-linux-gnu/14.3.0/include/immintrin.h:27,
                 from /home/coder/cuvs/cpp/build/conda/cuda-13.0/release/_deps/diskann-src/include/cosine_similarity.h:6,
                 from /home/coder/cuvs/cpp/build/conda/cuda-13.0/release/_deps/diskann-src/src/pq_flash_index.cpp:10:
/home/coder/.conda/envs/rapids/lib/gcc/x86_64-conda-linux-gnu/14.3.0/include/xsavesintrin.h: In function 'void _xsaves(void*, long long int)':
/home/coder/.conda/envs/rapids/lib/gcc/x86_64-conda-linux-gnu/14.3.0/include/xsavesintrin.h:41:3: error: '__builtin_ia32_xsaves' was not declared in this scope; did you mean '__builtin_ia32_xsavec'?
   41 |   __builtin_ia32_xsaves (__P, __M);
      |   ^~~~~~~~~~~~~~~~~~~~~
      |   __builtin_ia32_xsavec
compilation terminated due to -Wfatal-errors.

@achirkin
Copy link
Copy Markdown
Contributor

I confirm the affected benchmark builds and runs.

@achirkin
Copy link
Copy Markdown
Contributor

/merge

@rapids-bot rapids-bot Bot merged commit c515536 into rapidsai:main Oct 30, 2025
160 of 164 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Unstructured Data Processing Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cpp improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[BUG] Stop using private RMM macros in cuvs

2 participants