Uh oh!
There was an error while loading. Please reload this page.
cuda.core: allow updating Buffer deallocation streams - #2602
Conversation
This comment has been minimized.
This comment has been minimized.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| stream = Stream._from_handle(Stream, h_stream) if h_stream else default_stream() | ||
| if not h_stream: | ||
| print( | ||
| "Warning: no deallocation stream was recorded; falling back to " |
There was a problem hiding this comment.
Should it say "no deallocation stream was recorded or the deallocation stream was closed"?
There was a problem hiding this comment.
It's saying no stream is available for ordering the destruction. Not necessarily that a stream was provided and was closed.
There was a problem hiding this comment.
Yes, but wouldn't this warning also appear when the deallocation stream was closed?
There was a problem hiding this comment.
Two points: This warning does not occur if a valid stream is passed to set_deallocation_stream() and that stream is later closed, because the DevicePtrHandle holds an independent stream handle. It can occur if an already closed stream is passed.
The follow-up in #2635 should address the second case: closed streams will be rejected throughout the API, including by set_deallocation_stream(). If the proper checks are in place throughout the codebase, this condition should never occur, which is why I flagged it as an internal error.
There was a problem hiding this comment.
... because the DevicePtrHandle holds an independent stream handle.
What does invoking Stream.close mean then?
There was a problem hiding this comment.
A StreamHandle is a std::shared_ptr<CUstream>. A Python Stream class holds one of those, and Stream.close resets the shared pointer, dropping that reference. Inside a DevicePtrHandle is an independent StreamHandle referring to the deallocation stream. Whatever the user-facing Python Stream object does, the embedded reference cannot be invalidated.
Uh oh!
There was an error while loading. Please reload this page.
eef14b0 to
575c18fCompareShow how to move a Buffer between streams with an event so its eventual deallocation remains correctly ordered.
Keep the deallocation-stream tests aligned with the centralized test helper merged in NVIDIA#2624.
11cb846 to
7b04eacCompare| # The free operation runs after the copy on that stream. | ||
| buffer.close() | ||
| buffer = None | ||
| consumer_stream.sync() |
There was a problem hiding this comment.
This looks wrong: you close a buffer then sync a stream that has pending copy for the buffer. This sync could fail with a RuntimeError, couldn't it?
There was a problem hiding this comment.
The buffer in question has stream-ordered deallocation semantics, so closing it places a call to cuMemFreeAsync onto consumer_stream. The idea is that transferring the buffer to the stream "currently" using it will order its free after the work using it.
There was a problem hiding this comment.
Ah. For the other methods for stream-ordered operations stream is a mandatory parameter. That no stream is passed to close here caused me to think it is not an async operation.
| stream = Stream._from_handle(Stream, h_stream) if h_stream else default_stream() | ||
| if not h_stream: | ||
| print( | ||
| "Warning: no deallocation stream was recorded; falling back to " |
There was a problem hiding this comment.
Yes, but wouldn't this warning also appear when the deallocation stream was closed?
Uh oh!
There was an error while loading. Please reload this page.
This comment has been minimized.
This comment has been minimized.
3 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Summary
Closes#2600.
Add
Buffer.set_deallocation_streamso callers can replace the stream that orders eventual deallocation without closing the buffer. This supports transferring an allocation's lifetime to another stream while preserving the existingBufferobject.Changes
Buffer.set_deallocation_streamAPI and generated type stub.DeallocationStreamcapture path.Related Work
mainafter that PR merges.