Allow AGAIN while batching - #792
Open
bosilca wants to merge 5 commits into
Open
Conversation
A GPU submit hook can return PARSEC_HOOK_RETURN_AGAIN after it has collected several tasks into a batch ring. The retry path previously kept that ring attached to the head even though subsequent stream processing expects a singleton. This could lose the follower tasks or feed broken list linkage back into the queues. Treat batch collection as tentative until submission succeeds. Once the deferred event completes, detach the batch head, merge all followers back into the pending FIFO under one lock, and retry only the singleton head. Move the CUBLAS pool-capacity check in the DTD GEMM test after batch collection so this rollback path is exercised. Also document that parsec_list_item_ring_chop() reconnects the remaining ring but does not normalize the removed item, and singleton removed items in the LLP and private-scheduling paths before reusing their links. Finally, document next_task as a scheduler-bypassing private ready slot. A GPU manager can populate this slot while completing a GPU task and then remain in its progress loop, hiding the successor from other workers and reducing throughput. Flush the slot before entering GPU management and after every task completion so ready work remains visible to the scheduler. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
Batch collection removes follower tasks from the execution-stream FIFO before the submit hook returns. Paths that treated the returned wrapper as a singleton could strand followers, release too few wrappers, or corrupt the GPU manager outstanding count. Define ownership for every submit result. NEXT restores followers and keeps the existing singleton-head behavior. ASYNC transfers every execution context to the hook while the manager releases and accounts for every wrapper. ERROR and DISABLE quiesce the affected stream and clean the complete failed batch. Treat AGAIN as committed progress for the complete submitted ring. Keep that exact ring attached to the event and re-enter the submit hook with it after completion, allowing coroutine-style hooks to yield repeatedly. Make batch collection idempotent for such continuations and require predictable resource constraints to be checked before initial collection. Preserve the priority policy whenever singleton tasks or complete rings enter a stream FIFO. GPU wrappers carry a refreshed priority snapshot so the generic sorted-ring merge uses the correct object offset. Add explicit ACCEPT, REJECT, and successful STOP callback actions; invalid callback results become ERROR, and document that callbacks execute under the pending-FIFO lock. Document how hooks may manually disband an AGAIN ring and add CUDA regression coverage for repeated AGAIN continuation, ASYNC and NEXT ownership, priority ordering, callback STOP, and exact-once logical completion. Device-wide DISABLE recovery remains separate because all streams and queues must be reconciled together. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
An execution stream's next_task slot bypasses the scheduler to preserve private-task locality. The GPU submission path previously flushed this slot before manager election, even when another worker already managed the device and the submitting stream immediately returned to normal task selection. Move the entry flush after the existing GPU mutex election. Only the worker that changes the outstanding count from zero to one exposes its private task before entering long-lived GPU progress. Workers that increment an existing manager's count enqueue their GPU task and retain next_task for their own next selection. This keeps the single atomic election transition unchanged. Retain the post-completion flush because completing GPU work can reserve a CPU successor while the manager still owns other outstanding tasks. Document the private-slot policy at the execution-stream field and scheduler entry point, and record the remaining throughput benchmark in the GPU batch TODO. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
GPU execution profiling previously described only the selected head, while followers collected inside the submit hook had no task interval. PTG-generated hooks also emitted their own start before the common runtime knew the finalized batch, duplicating policy across submission paths. Move execution profiling into the common GPU runtime after a hook commits its ring with DONE or AGAIN. Emit one start for every logical member, retain each interval across repeated AGAIN continuations without progress markers, and emit one matching end at final completion. Keep the state on each wrapper so manually detached followers retain their accounting, and avoid rescanning committed non-singleton rings for new starts on every coroutine step. Separate deferred stage metadata from logical execution profiling: name the saved stage key and object ID explicitly, and derive execution end keys and taskpool IDs from each member's execution context. Preserve PTG body-level profiling policy with a compact DISABLED/PENDING/OPEN state. Document the lifecycle and extend the deterministic CUDA batch regression to verify that all members remain open across continuations and close before wrapper release. Mark complete profiling and the previously accepted next_task policy in the GPU review TODO. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
Document coroutine-style AGAIN continuation, complete-ring ASYNC handoff, and safe manual disbanding with repaired head linkage and priority-preserving follower requeue. Update the stage_custom batching example to use the named callback actions and stop once its batch is full. This avoids scanning and rejecting the rest of the pending FIFO while holding its lock. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
bosilca
force-pushed
the
topic/improved_gpu_again
branch
from
July 23, 2026 23:49
39b5a96 to
ff4adad
Compare
devreal
reviewed
Jul 24, 2026
|
|
||
| ```c | ||
| static void | ||
| requeue_batch_followers(parsec_gpu_exec_stream_t *gpu_stream, |
Contributor
There was a problem hiding this comment.
Could PaRSEC provide us with a function to do that? I don't like asking people to deal with internal data structures. Something like parsec_device_split_batch?
There was a problem hiding this comment.
🟡 Changes recommended
Two verified runtime issues remain: profiling stage-end traces can be skipped when logical execution ends, and the GPU failure path releases a failed batch without decrementing the device mutex counter.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates PaRSEC’s GPU batching semantics to allow PARSEC_HOOK_RETURN_AGAIN to preserve an intact batched ring across continuations, while also formalizing batch-collector callback actions (ACCEPT/REJECT/STOP), fixing priority-preserving stream insertion, and aligning profiling behavior with “one logical start/end per task” across repeated AGAIN.
Changes:
- Introduces explicit GPU batch-collector callback actions (
ACCEPT,REJECT,STOP) and updates batch collectors/tests accordingly. - Ensures batched rings are preserved across
AGAIN, followers are restored appropriately forNEXT, and batchedASYNCtransfers execution-context ownership to hooks. - Refactors GPU stream pending insertion to respect priority ordering consistently, and updates profiling to emit one start/end per logical task across
AGAIN.
File summaries
| File | Description |
|---|---|
| tests/runtime/cuda/stage_custom.jdf | Updates test batch-match callback to use explicit ACCEPT/REJECT/STOP actions and stop scanning once full. |
| tests/dsl/dtd/Testings.cmake | Adds a CUDA batching status regression test invocation with batching enabled. |
| tests/dsl/dtd/dtd_test_simple_gemm.c | Updates GEMM batching collector semantics and moves backpressure check before collection to match AGAIN ring preservation. |
| tests/dsl/dtd/dtd_test_cuda_again_async.c | Expands CUDA test coverage for batched AGAIN/ASYNC/NEXT, priority ordering, STOP behavior, and profiling invariants. |
| parsec/scheduling.c | Documents and strengthens next_task private-slot semantics (including singleton-ring invariants for flush). |
| parsec/mca/sched/llp/sched_llp_module.c | Ensures chopped single elements are re-singletoned to avoid stale/broken list links. |
| parsec/mca/device/device_gpu.h | Defines batch callback action enum and extends documentation on batching/ownership/profiling semantics. |
| parsec/mca/device/device_gpu.c | Implements AGAIN ring preservation, priority-aware pending insertion, batched ownership handling (NEXT/ASYNC/terminal errors), and profiling refactor. |
| parsec/interfaces/ptg/ptg-compiler/jdf2c.c | Shifts GPU execution profiling emission to common runtime; PTG sets wrapper profiling policy state. |
| parsec/include/parsec/execution_stream.h | Clarifies contract for the next_task private ready slot and when it must be flushed. |
| parsec/class/list_item.h | Updates documentation to reflect that ring_chop does not reinitialize the chopped item’s links. |
| docs/doxygen/task-batching.md | Updates batching documentation with the new callback actions, AGAIN/ASYNC/NEXT semantics, priority behavior, and profiling model. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+2337
to
+2340
| if( !keep_exec_open && | ||
| parsec_gpu_profile_exec_ring_end(stream, ring) ) { | ||
| return; | ||
| } |
Comment on lines
+3902
to
+3905
| if( NULL != failed_batch ) { | ||
| parsec_device_kernel_cleanout_ring(gpu_device, failed_batch); | ||
| (void)parsec_gpu_task_ring_release(failed_batch); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As an alternative to #790
This PR fixes GPU batch ownership across all submit-hook results.
AGAIN preserves the complete ring for coroutine-style continuation.
NEXT restores followers and retries the singleton head.
ASYNC transfers every execution context to the hook.
ERROR and DISABLE clean the complete failed batch.
Device-wide DISABLE recovery remains intentionally out of scope.