[None][perf] PyExecutor: skip empty MPI collectives in _fetch_new_requests (+ probe hang-detector fix)#14912
Conversation
|
/bot run --disable-fail-fast --add-multi-gpu-test |
|
PR_Github #51867 [ run ] triggered by Bot. Commit: |
|
PR_Github #51867 [ run ] completed with state
|
|
/bot run --disable-fail-fast --add-multi-gpu-test |
|
PR_Github #51926 [ run ] triggered by Bot. Commit: |
|
PR_Github #51926 [ run ] completed with state
|
When attention-DP is enabled, the KV-cache-aware router runs an allgather of per-request prefix-match lengths every iteration. The allgather still fires on an empty new_requests list, costing ~67 ms per empty iter from pure MPI collective rendezvous on DSv4 c=128. After _pop_from_waiting_queue, every rank holds an identical new_requests list (deterministic pop from a globally-broadcast waiting_queue), so the skip decision is rank-symmetric and cannot deadlock. Measured: 7% of steady-state iters on DSv4-Pro c128 have empty new_requests; skipping the allgather there saves ~4 ms/iter average. Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
request_broadcaster.broadcast() pickles new_requests + py_request_objects and runs two MPI_Bcasts even when rank 0's request queue is empty, costing ~58 ms per empty iter on DSv4 c=128 (~7% of iters in steady state). Add a tiny scalar broadcast first so all ranks symmetrically learn rank 0's len(new_requests) and can skip the heavy pickle path when it's zero. The probe is ~75 us, dwarfed by the saving on the skip path. Symmetric skip across all ranks cannot deadlock. py_request_objects is set to None on the skip path; downstream attach_py_objects_to_requests is already guarded by 'if py_request_objects'. Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The probe broadcast added by the previous commit (self.dist.broadcast(_new_request_count, root=0)) is not wrapped in hang_detector.pause(). When the server is idle, rank 0 blocks in get_from_request_queue(timeout=None) waiting for the next request -- that wait IS pause-wrapped -- while the non-root ranks skip the fetch and block on the probe broadcast waiting for rank 0. Because the probe is unpaused, the non-root ranks' 300s hang-detector watchdog falsely trips at the warmup->serving boundary (the first idle gap before requests flow), shutting the server down. Observed on DSv4-Pro disagg GEN (ranks 1-7 all stuck on comm.Bcast at the probe, 0 successful requests). Pre-probe, the equivalent non-root idle wait happened inside RequestBroadcaster.broadcast -> _broadcast_requests, whose non-root path is wrapped in hang_detector.pause(). The probe must do the same. Fix: wrap the probe broadcast in hang_detector.pause(), restoring the pre-change property that idle broadcast-waits do not trip the watchdog. Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5e5e54e to
95b147c
Compare
|
/bot run --disable-fail-fast --add-multi-gpu-test |
|
PR_Github #52001 [ run ] triggered by Bot. Commit: |
|
PR_Github #52001 [ run ] completed with state
|
|
/bot run --disable-fail-fast --add-multi-gpu-test |
|
PR_Github #52053 [ run ] triggered by Bot. Commit: |
|
PR_Github #52053 [ run ] completed with state
|
|
/bot run --disable-fail-fast --add-multi-gpu-test |
|
PR_Github #52078 [ run ] triggered by Bot. Commit: |
|
PR_Github #52078 [ run ] completed with state |
|
/bot run --disable-fail-fast --add-multi-gpu-test |
|
PR_Github #52133 [ run ] triggered by Bot. Commit: |
|
PR_Github #52133 [ run ] completed with state
|
Signed-off-by: peihengh <259410613+peihu-nv@users.noreply.github.com>
|
/bot run --disable-fail-fast --add-multi-gpu-test |
|
PR_Github #52149 [ run ] triggered by Bot. Commit: |
|
PR_Github #52149 [ run ] completed with state
|
|
/bot run |
|
PR_Github #52185 [ run ] triggered by Bot. Commit: |
|
CI failures are unrelated node issues. x86 single hit the known DS B200 KV-transfer failure that hitting multiple PRs. Skipping CI and merging. |
|
PR_Github #52185 [ run ] completed with state |
…uests (+ probe hang-detector fix) (NVIDIA#14912) Source-Commit: 49f1ce6ce5d6847af7217e3cf5ba3ff9f5e2fdb2 Signed-off-by: Jiagan Cheng <jiaganc@nvidia.com>
…uests (+ probe hang-detector fix) (NVIDIA#14912) Source-Commit: 49f1ce6ce5d6847af7217e3cf5ba3ff9f5e2fdb2 Signed-off-by: Jiagan Cheng <jiaganc@nvidia.com>
Summary
Three changes to
PyExecutor._fetch_new_requeststhat cut empty-iter MPI collective overhead on the host critical path, plus a hang-detector fix for the new probe.Commits:
[perf] skip empty gather_prefix_matches(F1) — guard the KV-cache-aware prefix-match allgather onnew_requestsbeing non-empty.[perf] probe before heavy request broadcast(F2) — broadcast a 1-int probe of rank-0's count before the pickle-heavyrequest_broadcaster.broadcast()so all ranks symmetrically skip when there is nothing to send.[fix] pause hang detector around request-count probe— wrap F2's probe broadcast inhang_detector.pause()(see "Hang-detector fix" below).Both skips are rank-symmetric (they branch on data already identical across ranks after the prior
_pop_from_waiting_queue/ probe), so they cannot deadlock.Measurements
DSv4-Pro, CTX-only, agentperf-v2, DEP=4 + attention-DP + KV-cache-aware routing, c=128, 1 node x 4 GB200.
The probe-pause commit only touches the idle watchdog path, so steady-state throughput is unchanged from F1+F2 (the +4.4% vs +5.3% is run-to-run variance, ±2-3%).
Per-NVTX-range (100 captured iters, rank 0):
_fetch_new_requests(parent)broadcast_payload(heavy pickle)adp_gather_prefix_matchesHang-detector fix (commit 3) — important for disagg
F2's probe
self.dist.broadcast(_new_request_count, root=0)was NOT wrapped inhang_detector.pause(). When the server is idle, rank 0 blocks inget_from_request_queue(timeout=None)(which IS pause-wrapped) waiting for the next request, while the non-root ranks skip the fetch and block on the probe broadcast waiting for rank 0. With the probe un-paused, the non-root ranks' 300s hang-detector watchdog falsely trips at the warmup->serving boundary and shuts the server down.Pre-PR, that same non-root idle wait happened inside
RequestBroadcaster.broadcast->_broadcast_requests, whose non-root path is wrapped inhang_detector.pause(). F2 moved the wait out of a paused region; commit 3 restores it.Reproduced on a DSv4-Pro disagg GEN server (DEP8). A/B on the same cluster, same workload, identical except the one-line wrap:
comm.Bcastat the probe; server self-terminatesWhy F1 / F2 are safe
After
_pop_from_waiting_queue, every rank holds an identicalnew_requestslist (deterministic pop from a globally-broadcast waiting_queue), solen(new_requests) == 0(F1) and the probe count (F2) are rank-symmetric: all ranks either skip together or execute together. On F2's skip pathpy_request_objectsisNone; downstreamattach_py_objects_to_requestsis already guarded byif py_request_objects. F2's probe uses the sameself.dist.broadcast(int, root=0)idiom as the pre-existing_sync_and_process_resource_governor_queue.Test plan
Related
Companion 1-line PR #14889 (
copy.deepcopy->list()) gives +11.7% on the same workload; F1+F2 stacks with it for +13.4%.🤖 Generated with Claude Code