Skip to content

Allow construction of CudaStreamPool from shared_ptr#2447

Open
wence- wants to merge 2 commits into
rapidsai:mainfrom
wence-:wence/fix/2246
Open

Allow construction of CudaStreamPool from shared_ptr#2447
wence- wants to merge 2 commits into
rapidsai:mainfrom
wence-:wence/fix/2246

Conversation

@wence-

@wence- wence- commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Description

Checklist

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

@wence- wence- requested a review from a team as a code owner June 17, 2026 11:34
@wence- wence- requested a review from bdice June 17, 2026 11:34
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3eb9301c-eea0-4ddb-9fbf-26af87ae2fda

📥 Commits

Reviewing files that changed from the base of the PR and between a4ab399 and 960cdf8.

📒 Files selected for processing (3)
  • python/rmm/rmm/librmm/memory_resource.pxd
  • python/rmm/rmm/pylibrmm/cuda_stream_pool.pxd
  • python/rmm/rmm/pylibrmm/cuda_stream_pool.pyx

📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Optimized internal memory resource threading declarations for improved performance
    • Enhanced stream pool object initialization with new factory utilities

Walkthrough

Adds a CudaStreamPool.c_from_shared_ptr static factory method to the Cython .pxd interface and .pyx implementation, enabling construction of a CudaStreamPool wrapper by moving an existing shared_ptr[cuda_stream_pool]. Also marks two cdef extern from * blocks in memory_resource.pxd as nogil.

Changes

CudaStreamPool.c_from_shared_ptr factory method

Layer / File(s) Summary
c_from_shared_ptr declaration and implementation
python/rmm/rmm/pylibrmm/cuda_stream_pool.pxd, python/rmm/rmm/pylibrmm/cuda_stream_pool.pyx
Declares c_from_shared_ptr as a @staticmethod cdef in the .pxd interface. The .pyx implementation imports move, validates the pointer is non-null (raising ValueError), allocates a CudaStreamPool via __new__, moves ownership of the shared_ptr[cuda_stream_pool] into obj.c_obj, and returns the instance.

memory_resource.pxd nogil extern blocks

Layer / File(s) Summary
nogil annotations on helper extern blocks
python/rmm/rmm/librmm/memory_resource.pxd
The cdef extern from * block hosting make_any_device_resource and related helpers, and the block containing the failure_callback_resource_adaptor_oom overload, are both changed to cdef extern from * nogil. One inner make_device_async_resource_ref declaration drops its now-redundant explicit nogil modifier.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • rapidsai/rmm#2432: Switches CudaStreamPool.c_obj to shared_ptr and updates construction logic, directly preceding this PR's addition of c_from_shared_ptr.
  • rapidsai/rmm#2436: Touches the same memory_resource.pxd Cython extern declarations for any_resource/device_async_resource_ref, indicating shared migration work.

Suggested labels

non-breaking, improvement

Suggested reviewers

  • bdice
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: enabling CudaStreamPool construction from shared_ptr, which is the primary feature added in this PR.
Description check ✅ Passed The description is related to the changeset, referencing the closed issue #2446 and confirming adherence to contributing guidelines, tests, and documentation updates.
Linked Issues check ✅ Passed The PR implementation directly addresses issue #2446 by adding the c_from_shared_ptr static method to construct CudaStreamPool from shared_ptr[cuda_stream_pool], meeting the stated requirement.
Out of Scope Changes check ✅ Passed All changes are in scope: modifications to memory_resource.pxd add nogil declarations (Cython quality improvement), while cuda_stream_pool files implement the requested shared_ptr construction feature.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@wence- wence- added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[FEA] Allow constructing CudaStreamPool from shared_ptr[cuda_stream_pool]

1 participant