[EASY][STF] host_launch_scope, parallel_for_scope: timing events are released under a policy, not an abort - #11773
Conversation
…ed under a policy, not an abort The SCOPE(exit) that destroys the two timing events in each operator->* becomes cuda_try under ON_THROW(notify), one guard per release, as in launch (NVIDIA#11748) and stream_task (NVIDIA#11770): a failing destroy leaks that event and is reported, the guard is noexcept so it cannot become a second exception, and a leaked event is not worth ending the program over. The timing blocks in SCOPE(success) are left to NVIDIA#11652. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
/ok to test a0cd585 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 SummarySummary by CodeRabbit
WalkthroughThe host launch and parallel-for scope cleanup guards now destroy each non-null timing event with a separate ChangesTiming event cleanup
Priority: ⬇️ Low Change: Bug fix Merge Risk: 🔵 Low · up to The PR is mergeable with a small follow-up: qualify the event-cleanup calls as required by the project guidelines. No runtime impact is established.
Comment ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cudax/include/cuda/experimental/__stf/internal/host_launch_scope.cuh (1)
237-237: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFully qualify the new event-release calls.
Use
::cuda::experimental::stf::cuda_try<::cudaEventDestroy>at each site.
cudax/include/cuda/experimental/__stf/internal/host_launch_scope.cuh#L237-L237: qualify this call and the matching call at Line 244.cudax/include/cuda/experimental/__stf/internal/parallel_for_scope.cuh#L705-L705: qualify this call and the matching call at Line 712.As per path instructions, “free-function calls must be fully qualified to avoid ADL.”
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: NVIDIA/cccl/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f92f8b54-7482-43e4-9c8e-6a79d6c11fae
📒 Files selected for processing (2)
cudax/include/cuda/experimental/__stf/internal/host_launch_scope.cuhcudax/include/cuda/experimental/__stf/internal/parallel_for_scope.cuh
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
This comment has been minimized.
This comment has been minimized.
🥳 CI Workflow Results🟩 Finished in 2h 20m: Pass: 100%/72 | Total: 2d 11h | Max: 1h 19m | Hits: 9%/446354See results here. |
2 participants
[EASY]
Description
Same change as #11770 (stream_task) and the event part of #11748 (launch), applied to the two remaining copies of the pattern: the
SCOPE(exit)inhost_launch_scope.cuhand inparallel_for_scope.cuhthat destroys the two timing events of anoperator->*.Each
cuda_safe_call(cudaEventDestroy(...))becomescuda_try<cudaEventDestroy>underON_THROW(notify), one guard per release. A failing destroy leaks that event and is reported to stderr; the guard isnoexcept, so it cannot become a second exception; a leaked event is not worth ending the program over, which is whatabort()did; and one policy per release means a failure on the first destroy does not skip the second.The
SCOPE(success)timing blocks in both files are left to #11652, which replaces them withtask_statistics::record_task_timing.With this, all five copies of the timing-event release guard carry the same explicit policy; folding them into one RAII holder is the follow-up once #11652 lands.
Checklist
🤖 Generated with Claude Code