hipfile: feat(hipfile/python): add async stream I/O bindings - #7386
Conversation
Expose the hipFile async API to the Python bindings. The C API (hipFileReadAsync / hipFileWriteAsync / hipFileStreamRegister / hipFileStreamDeregister) already exists; only the Python layer was missing -- the bindings previously had just the hipFileAsyncNotSupported enum. - _chipfile.pxd: declare hipStream_t + the four async functions. - _hipfile.pyx: AsyncIOHandle cdef class that owns the in/out C slots (size / file_offset / buffer_offset / bytes_done) so their addresses stay valid until the stream completes. The driver dereferences these pointers AFTER the async call returns (the transfer finishes on stream sync), so a plain Cython stack-local passed by address would be written back to a dead address -- bytes_done reads 0 and later submits can hit hipFileInvalidValue. Plus read/write async wrappers, stream register/deregister, and a supports_async() probe. - file.py: FileHandle.read_async / write_async returning an AsyncIOHandle (keep alive past stream sync, then read bytes_done), a Stream context manager, and supports_async(). - __init__.py: export Stream and supports_async. Mirrors the cuFile async pattern. Validated on gfx942 / ROCm 7.2: build + import, supports_async() == True, and a GPU async write->read round-trip (bytes_done == 4096, data byte-identical).
|
Why we need it With the synchronous hipFileRead, each chunk's disk→GPU load blocks the calling thread and the GPU sits idle for the duration of the I/O — the load is serialized against compute. At serving scale that I/O wait dominates the step (we saw multi-second per-step H2D stalls and a pacing-bound throughput plateau). The fix is the standard GPUDirect-Storage overlap pattern: submit the read on a side stream and let it run concurrently with the model forward pass, gating each layer's attention on just its chunk via a HIP event — exactly what cuFileReadAsync enables on NVIDIA. The hipFile C async API already does this; only the Python layer was missing. This PR adds it so our Python connector can use the async path instead of a ctypes shim. Usage — layerwise load overlapped with compute (our actual pattern) The one correctness gotcha worth a binding (why AsyncIOHandle exists) Following is how it fixed: The slots are C members of a cdef class, so their addresses are stable for the whole lifetime of the Python object — not stack-locals that die when the wrapper returns: So the driver's deferred write to bytes_read_p lands in a valid address as long as the AsyncIOHandle is alive. Validated (gfx942 / ROCm 7.2) |
There was a problem hiding this comment.
Pull request overview
This PR adds Python bindings for hipFile’s asynchronous stream-attached I/O API so Python users can submit hipFileReadAsync/hipFileWriteAsync operations and manage stream registration from the Python layer (mirroring cuFile’s async pattern).
Changes:
- Add Cython declarations and wrappers for
hipFileReadAsync,hipFileWriteAsync,hipFileStreamRegister, andhipFileStreamDeregister, plus an async support probe. - Introduce
AsyncIOHandleto keep async in/out slot storage alive across stream execution. - Add high-level Python
FileHandle.read_async/write_async,Streamregistration context manager, and exportStream/supports_asyncfrom the package.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| projects/hipfile/python/hipfile/file.py | Adds high-level async read/write APIs and a stream registration helper class. |
| projects/hipfile/python/hipfile/_hipfile.pyx | Adds Cython async wrappers, AsyncIOHandle, stream register/deregister wrappers, and an async capability probe. |
| projects/hipfile/python/hipfile/_chipfile.pxd | Declares hipStream_t and async/stream APIs in the Cython pxd layer. |
| projects/hipfile/python/hipfile/init.py | Exports Stream and supports_async at the package top level. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
riley-dixon
left a comment
There was a problem hiding this comment.
Hi @jiejingzhangamd ,
Thank you for the contribution to the hipFile Python bindings!
There are a couple of things I just want to mention to make sure you are aware of:
-
The Async API only supports the POSIX fallback path at this moment in time. Async support for the GPU optimized path is planned for in the future.
-
The long-term plan is to migrate these Python bindings into HIP-Python. When that happens, the internals of AsyncIOHandle will be reworked to use HIP-Python Pointer objects for storage rather than the current Cython implementation. I believe though the public API contract for
read_asyncandwrite_asynccould be maintained. -
If this sounds reasonable to you, I can update the hipfile PyPI package after these changes have been merged.
Please let me know your thoughts!
- errno only for POSIX/C errors (err == -1) in the async read/write wrappers, and also capture it for stream register/deregister; other hipFileOpError_t codes leave extra=0 (errno would be stale). Mirrors the sync path. - read_async/write_async now raise OSError on err == -1 (consistent with read()/write() and the hipFile error contract), HipFileException otherwise. - supports_async(): deregister the default stream if the probe registered it, so the probe leaves no permanent registration behind. - Stream.deregister(): only clear _registered after a successful deregister. - AsyncIOHandle: add size/file_offset/buffer_offset setters (async API allows modifying these after submission); fix docstring (no Stream.synchronize — synchronise the underlying HIP/CUDA stream / wait on an event).
|
Thanks for the review! Pushed fixes:
On the roadmap — the HIP-Python migration and you updating the PyPI package after merge both sound good to me. |
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
The Async Python API was added after the initial unit tests were merged, but the Async PR (#7386) did not add any unit tests itself. This quick fix addresses the import issue faced. Actual unit tests for async will be added at a later point.
## Motivation The hipFile Python test suite has been broken at collection since the async I/O bindings (#7386): that PR added `AsyncIOHandle` and the async/stream callables to the extension and imported them in `file.py`, but never updated the fake in `conftest.py`. Every test importing `hipfile` errored during collection. Missed because CI wasn't re-run after the unit tests (#8725) landed. ## Technical Details Add the missing async names to the fake extension in `conftest.py`: a `_FakeAsyncIOHandle` stand-in plus success-shaped `hipFileReadAsync` / `hipFileWriteAsync` / `hipFileStreamRegister` / `hipFileStreamDeregister` / `supports_async`. Tests-only change. ## Issue Tracking JIRA ID: AIHIPFILE-171 ## Test Plan - Run the hipFile Python binding suite: `pytest projects/hipfile/python/tests/`. - Lint the changed file: `black --check` and `pylint` on `conftest.py`. ## Test Result - `42 passed` (previously: 5 collection errors, suite could not run). - `black --check`: file left unchanged. `pylint`: rated 10.00/10. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
|
Hi @jiejingzhangamd - just want to let you know that the hipfile package on PyPI has been updated to 0.4.0.dev0 which includes your async changes! |
Expose the hipFile async API to the Python bindings. The C API (hipFileReadAsync / hipFileWriteAsync / hipFileStreamRegister / hipFileStreamDeregister) already exists; only the Python layer was missing -- the bindings previously had just the hipFileAsyncNotSupported enum.
Mirrors the cuFile async pattern. Validated on gfx942 / ROCm 7.2: build + import, supports_async() == True, and a GPU async write->read round-trip (bytes_done == 4096, data byte-identical).
Motivation
Technical Details
JIRA ID
AIHIPFILE-171
Test Plan
Test Result
Submission Checklist