Make GraphicsResource inherit from Buffer

[rparolin]

Summary

• Refactors GraphicsResource to inherit from Buffer instead of wrapping a separate Buffer object, eliminating the _MappedBufferContext intermediary class
• map() now populates the GraphicsResource itself with device pointer/size (since it IS-A Buffer) and returns self
• Makes stream a required parameter for map() and unmap() (removes implicit default stream usage)
• Adds stream parameter to from_gl_buffer() for a convenient register-and-map-in-one-step pattern
• Renames handle property to resource_handle to avoid conflicting with Buffer.handle (which now exposes the mapped device pointer)

Motivation

Review feedback requested that GraphicsResource be a Buffer directly rather than producing a separate Buffer via map(). This simplifies the API surface: a mapped GraphicsResource can be passed directly anywhere a Buffer is accepted (e.g., StridedMemoryView.from_buffer()), without needing a wrapper context manager.

Key changes

• GraphicsResource(Buffer) — inherits from Buffer; handle and size are valid while mapped
• Removed _MappedBufferContext — no longer needed since GraphicsResource itself is the buffer
• __enter__/__exit__ — moved onto GraphicsResource directly; __exit__ auto-unmaps using the stream from map()
• stream required — map(stream=) and unmap(stream=) no longer default to stream 0
• from_gl_buffer(stream=) — optional stream param to register + map in one call, enabling with GraphicsResource.from_gl_buffer(vbo, stream=s) as buf: pattern
• close(stream=None) — accepts stream kwarg for compatibility with Buffer.close()
• resource_handle — renamed from handle to avoid shadowing Buffer.handle

🤖 Generated with Claude Code

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[copy-pr-bot]

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[rparolin]

/ok to test

[github-actions]

Doc Preview CI
🚀 View preview at https://nvidia.github.io/cuda-python/pr-preview/pr-1701/
https://nvidia.github.io/cuda-python/pr-preview/pr-1701/cuda-core/
https://nvidia.github.io/cuda-python/pr-preview/pr-1701/cuda-bindings/
https://nvidia.github.io/cuda-python/pr-preview/pr-1701/cuda-pathfinder/
Preview will be ready when the GitHub Pages deployment is complete.

[rparolin]

/ok to test

[cpcloud]

Potential stream-ordering bug here: map()/unmap() now require an explicit stream, but close() always unmaps on stream 0 and ignores both _map_stream and the optional close(stream=...) arg.

If a resource was mapped on a non-default stream and then closed while mapped, unmap can be issued on the wrong stream, which may break ordering guarantees with in-flight work. Could we unmap using _map_stream (or the passed stream, with a clear fallback policy) instead of hard-coding stream 0?

[cpcloud]

__enter__ currently returns self even when the resource is not mapped. That allows patterns like with GraphicsResource.from_gl_buffer(vbo) as buf: (without stream=), where buf.handle/buf.size are not valid.

Could we either (a) raise in __enter__ when not self._mapped, or (b) require/perform mapping for the context-manager path to avoid this silent footgun?

[cpcloud]

Thanks for the refactor here — the GraphicsResource(Buffer) direction looks good overall.

Requesting changes for two behavior issues called out inline:

1. close() currently unmaps on stream 0 even when mapping happened on a non-default stream, which can break stream-ordering guarantees.
2. __enter__ returns self even when unmapped, allowing context-manager usage where handle/size are invalid.

Please address these and I’m happy to re-review.

[cpcloud]

ping @rparolin. happy to fix this up if you want.

[leofang]

Thanks for the refactor here — the GraphicsResource(Buffer) direction looks good overall.

Yeah and thinking about it more, I suspect this is a must:

• If GL buffer registration is context-independent: This PR makes the UX nicer
• If GL buffer registration is locked to the current CUDA context: We need to track the underlying device/context, so we either inherit from Buffer (this PR) or add Buffer.from_gl_buffer() constructor (and keep GraphicsResource internal).
  • From the viewpoint of staying consistent across cuda.core, the latter might be worth considering too, but one challenge is that Buffer today does not have the map/unmap concept.
  • I would be happy for you @rparolin @Andy-Jost @cpcloud to brainstorm and I can review later 🙂

I will ask internally if it's context-independent or not, but it's not a blocker.

[Andy-Jost]

Thanks for the discussion here. I'd like to propose an alternative approach that I think addresses both of Phillip's concerns and Leo's observation about Buffer not having map/unmap.

Proposal: Use the resource_handles module instead of modifying Buffer

We already have a C++ handle layer (cuda/core/_cpp/resource_handles.hpp) that provides RAII-based lifetime management with std::shared_ptr custom deleters. This pattern already solves the exact problems raised here:

1. Stream capture: DevicePtrHandle already captures the allocation stream in its deleter and uses it for cuMemFreeAsync. We can do the same for graphics resources - capture the map stream and use it for unmap.

2. Dependency tracking: Handles can hold references to other handles, preventing out-of-order destruction. A mapped buffer handle can hold a reference to the GraphicsResourceHandle, ensuring the graphics resource outlives the mapped pointer.

3. Arbitrary release actions: The shared_ptr deleter can chain actions - e.g., unmap then unregister.

Concrete design:

• GraphicsResource.map(stream) returns a Buffer (or just the existing DevicePtrHandle under the hood)
• The returned handle's deleter captures:
  • The GraphicsResourceHandle (keeps it alive)
  • The map StreamHandle (for stream-ordered unmap)
• When the mapped buffer handle is released, the deleter calls cuGraphicsUnmapResources on the captured stream
• GraphicsResource itself doesn't need to track _mapped state - the existence of mapped buffer handles implies mapped state

Benefits:

• Buffer stays unchanged (no map/unmap concept needed)
• Stream ordering is correct by construction (cpcloud's concern 1)
• No invalid __enter__ state possible (cpcloud's concern 2) - you either have a valid mapped handle or you don't
• Dependency tracking prevents use-after-free
• Consistent with how we handle DeviceMemoryResource allocations

This is the same pattern we use for memory pool allocations, where the pointer handle captures the pool handle and stream for proper cleanup ordering.

Thoughts?

[cpcloud]

+1 to @Andy-Jost's design. Looks solid.

[leofang]

FYI

I will ask internally if it's context-independent or not

it is context-dependent.

Proposal: Use the resource_handles module instead of modifying Buffer

Modifying Buffer in what way? Sorry it's not clear to me.

Can we sketch a bit how the user code would look like with this proposal? The changes in this PR were motivated by the sketch in here: #1608 (comment). Rob asked the right question there. Inheritance is probably not necessary, as long as the same UX can be delivered.