cudax/stf: cuda_try migration — stream_task (PR9)#10003
Conversation
Migrate throwing-capable stream task setup to cuda_try while retaining cuda_safe_call in noexcept scope guards. Make timing-event ownership exception-safe and use the capture-aware stream-device helper so partial failures restore device state and release events.
|
/ok to test caf42e7 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesSTF timing paths now use conditionally created CUDA events with RAII cleanup and explicit success/failure handling. Stream dependency insertion is device-scoped, launch cleanup covers temporary and cooperative-group allocations, and graph task timing decisions combine scheduling with calibration state. STF timing and cleanup
Possibly related PRs
Suggested labels: Suggested reviewers: Comment |
Destroy timing events reliably and separate successful timing collection from failure cleanup across stream and launch tasks.
|
/ok to test 19bb001 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cudax/include/cuda/experimental/__stf/internal/launch.cuh (1)
399-403: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winimportant: pass
grid_size - 1todeallocateManagedMemoryheredeallocateManagedMemorykeys the pool bysz, so freeing theallocateManagedMemory(grid_size - 1)buffer withgrid_sizereinserts it under the wrong size and can later return a short buffer for agrid_sizeallocation.
🧹 Nitpick comments (1)
cudax/include/cuda/experimental/__stf/internal/launch.cuh (1)
389-389: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuesuggestion: Declare
free_launch_tempsasconst; it is never reassigned. As per coding guidelines, “All variables that are not modified must be declaredconst.”Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 56b6c5a9-8b62-4e1d-8591-f6c5a43c262b
📒 Files selected for processing (4)
cudax/include/cuda/experimental/__stf/graph/graph_task.cuhcudax/include/cuda/experimental/__stf/internal/host_launch_scope.cuhcudax/include/cuda/experimental/__stf/internal/launch.cuhcudax/include/cuda/experimental/__stf/stream/stream_task.cuh
Build the execution policy before starting the task so teardown guards cover occupancy failures, adopt arrived-list ownership atomically, and install the device-temp free guard before set_device_tmp.
The 3-arg constructor forwarded arg and streams in the wrong order to the primary constructor.
|
/ok to test f89181b |
A SCOPE(fail) guard only runs during unwinding, so it can directly free an unadopted arrived list without tracking redundant state.
|
/ok to test 7e45b83 |
😬 CI Workflow Results🟥 Finished in 1h 24m: Pass: 28%/57 | Total: 1d 11h | Max: 1h 24m | Hits: 14%/30082See results here. |
Summary
stream_task.cuhfromcuda_safe_calltocuda_trytiming_activestateTest plan
pre-commit run --files cudax/include/cuda/experimental/__stf/graph/graph_task.cuh cudax/include/cuda/experimental/__stf/internal/host_launch_scope.cuh cudax/include/cuda/experimental/__stf/internal/launch.cuh cudax/include/cuda/experimental/__stf/stream/stream_task.cuhninja -j 12 -C build/cudax-cpp20 cudax.test.stf.dot.with_events cudax.test.stf.cpp.task_get_stream cudax.test.stf.interface.host_launch_deps cudax.test.stf.stress.launch_overhead cudax.test.stf.places.execution_policy_kernel_launch_testctest --test-dir build/cudax-cpp20 -j 12 -R 'cudax\.test\.stf\.(examples\.01-axpy-places|places\.(cuda_stream_place|non_current_device))' --output-on-failureCUDASTF_DOT_TIMING=1 ./build/cudax-cpp20/bin/cudax.test.stf.examples.01-axpy-places