Eliminate _CXX_API capsule for resource handle functions #1464
Conversation
Replace the PyCapsule-based function pointer table with direct Cython cimport. Consumer modules now call resource handle functions directly through _resource_handles.so, simplifying the architecture while correctly sharing static/thread-local state. Changes: - Remove _CXX_API capsule infrastructure (resource_handles_cxx_api.hpp, _resource_handles_cxx_api.pxd, get_resource_handles_cxx_api_v1()) - Remove _init_handles_table() calls from all consumer modules - Rename create_event_handle(flags) to create_event_handle_noctx(flags) to avoid C++ overload ambiguity for Cython binding - Update DESIGN.md to reflect the simplified architecture - Add clarifying comment in build_hooks.py for cpp file discovery Closes NVIDIA#1452
Andy-Jost
added
enhancement
|
/ok to test 6ec8330 |
|
|
/ok to test bcefde1 |
rwgk
left a comment
rwgk
left a comment
There was a problem hiding this comment.
That's a great simplification! Nothing stood out to me looking through visually. Cursor found one minor inconsistency.
|
|
||
| # Context handles | ||
| ContextHandle create_context_handle_ref "cuda_core::create_context_handle_ref" ( | ||
| cydriver.CUcontext ctx) nogil |
There was a problem hiding this comment.
Cursor discovered this:
Minor Issue: Inconsistent noexcept Annotations
The .pxd file declares all handle functions as noexcept nogil:
# _resource_handles.pxd
cdef ContextHandle create_context_handle_ref(cydriver.CUcontext ctx) noexcept nogil
cdef StreamHandle create_stream_handle(...) noexcept nogil
cdef EventHandle create_event_handle(...) noexcept nogil
# etc.But the .pyx file's cdef extern from declarations are missing noexcept on many of these same functions:
# _resource_handles.pyx
ContextHandle create_context_handle_ref "cuda_core::create_context_handle_ref" (
cydriver.CUcontext ctx) nogil # <-- missing noexcept
StreamHandle create_stream_handle "cuda_core::create_stream_handle" (
ContextHandle h_ctx, unsigned int flags, int priority) nogil # <-- missing noexcept
EventHandle create_event_handle "cuda_core::create_event_handle" (
ContextHandle h_ctx, unsigned int flags) nogil # <-- missing noexceptMeanwhile, some functions do have noexcept in both places (e.g., get_primary_context, get_current_context, get_legacy_stream, get_per_thread_stream).
Functions missing noexcept in .pyx:
create_context_handle_refcreate_stream_handlecreate_stream_handle_refcreate_event_handlecreate_event_handle_noctxcreate_event_handle_ipccreate_mempool_handlecreate_mempool_handle_refcreate_mempool_handle_ipcdeviceptr_alloc_from_pooldeviceptr_alloc_asyncdeviceptr_allocdeviceptr_alloc_hostdeviceptr_create_refdeviceptr_import_ipc
Impact: The C++ functions don't throw exceptions, so noexcept is semantically correct for all of them. The inconsistency probably doesn't cause runtime issues, but it would be cleaner if the .pyx declarations matched the .pxd declarations exactly.
Suggested fix: Add noexcept to all the cdef extern from declarations in _resource_handles.pyx that have it in _resource_handles.pxd.
2 participants
Summary
cimport_resource_handles.soChanges
_CXX_APIcapsule infrastructure (resource_handles_cxx_api.hpp,_resource_handles_cxx_api.pxd,get_resource_handles_cxx_api_v1())_init_handles_table()calls from all consumer modulescreate_event_handle(flags)tocreate_event_handle_noctx(flags)to avoid C++ overload ambiguity for Cython bindingDESIGN.mdto reflect the simplified architecturebuild_hooks.pyfor cpp file discoveryTest Plan
resource_handles.cppsymbols exist only in_resource_handles.so(confirmed vianm -C)Stats
16 files changed, 179 insertions(+), 512 deletions(-)
Net reduction: 333 lines
Closes #1452