Repository navigation
Host overhead 6/N: declared stop policies, checked in the graph runtime - #857
Conversation
af7d533 to
99156cb
Compare
99156cb to
a828365
Compare
a828365 to
ef863a7
Compare
ef863a7 to
6e983f5
Compare
6e983f5 to
b5705cf
Compare
14ce3d3 to
d01f62a
Compare
8022de6 to
e9413ac
Compare
route_and_send_inline is gone from #857; the native postprocess routes inside allow_threads the same way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#857's postprocess takes a step's worker-checked stops in the request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
e9413ac to
5c6e575
Compare
route_and_send_inline is gone from #857; the native postprocess routes inside allow_threads the same way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#857's postprocess takes a step's worker-checked stops in the request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| /// Whether this request's current walk actually contains the loop. A stop | ||
| /// for one it does not is a model bug: logged by the caller and dropped. | ||
| /// `rid`'s iteration of loop `lp` in partition `p`, as | ||
| /// `get_dynamic_loop_iters` reports it (the last worker graph with the | ||
| /// loop wins); 0 when no live worker graph has it. | ||
| fn loop_count(&self, rid: u32, p: Sym, lp: Sym) -> u32 { | ||
| let Some(part) = self.info(rid).and_then(|i| i.partitions.get(&p)) else { return 0 }; | ||
| let mut count = 0; | ||
| for &wg in &part.walk_worker_graphs { | ||
| let Some(state) = self.state(wg, rid) else { continue }; | ||
| if let Some(&lid) = self.graphs[wg as usize].loop_by_name.get(&lp) { | ||
| count = state.loop_iter(lid); | ||
| } | ||
| } | ||
| count | ||
| } | ||
|
|
||
| fn check_dyn_loop(&self, rid: u32, partition: &str, loop_name: &str) -> bool { |
There was a problem hiding this comment.
[P2] loop_count was added between check_dyn_loop and its doc comment, so check_dyn_loop's doc now sits on loop_count.
The rustdoc for loop_count now begins "Whether this request's current walk actually contains the loop. A stop for one it does not is a model bug", which isn't what it does, and check_dyn_loop is left with no doc.
| /// Whether this request's current walk actually contains the loop. A stop | |
| /// for one it does not is a model bug: logged by the caller and dropped. | |
| /// `rid`'s iteration of loop `lp` in partition `p`, as | |
| /// `get_dynamic_loop_iters` reports it (the last worker graph with the | |
| /// loop wins); 0 when no live worker graph has it. | |
| fn loop_count(&self, rid: u32, p: Sym, lp: Sym) -> u32 { | |
| let Some(part) = self.info(rid).and_then(|i| i.partitions.get(&p)) else { return 0 }; | |
| let mut count = 0; | |
| for &wg in &part.walk_worker_graphs { | |
| let Some(state) = self.state(wg, rid) else { continue }; | |
| if let Some(&lid) = self.graphs[wg as usize].loop_by_name.get(&lp) { | |
| count = state.loop_iter(lid); | |
| } | |
| } | |
| count | |
| } | |
| fn check_dyn_loop(&self, rid: u32, partition: &str, loop_name: &str) -> bool { | |
| /// `rid`'s iteration of loop `lp` in partition `p`, as | |
| /// `get_dynamic_loop_iters` reports it (the last worker graph with the | |
| /// loop wins); 0 when no live worker graph has it. | |
| fn loop_count(&self, rid: u32, p: Sym, lp: Sym) -> u32 { | |
| let Some(part) = self.info(rid).and_then(|i| i.partitions.get(&p)) else { return 0 }; | |
| let mut count = 0; | |
| for &wg in &part.walk_worker_graphs { | |
| let Some(state) = self.state(wg, rid) else { continue }; | |
| if let Some(&lid) = self.graphs[wg as usize].loop_by_name.get(&lp) { | |
| count = state.loop_iter(lid); | |
| } | |
| } | |
| count | |
| } | |
| /// Whether this request's current walk actually contains the loop. A stop | |
| /// for one it does not is a model bug: logged by the caller and dropped. | |
| fn check_dyn_loop(&self, rid: u32, partition: &str, loop_name: &str) -> bool { |
Automated review🟡 3 P2 on Four of the five earlier findings are fixed (combined-walk output signals, Orpheus/Whisper stop buffers, stream_partition_done after a dropped row, nogil accounting), and the GIL-release P1 is addressed: the Rust postprocess now holds the GIL except for an inline step's route and send. One new P2 is posted, on a misplaced Rust doc comment. Description
What the review checked
Advisory, not a merge gate. Rules: |
route_and_send_inline is gone from #857; the native postprocess routes inside allow_threads the same way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#857's postprocess takes a step's worker-checked stops in the request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
d399cd5 to
6b35e14
Compare
6b35e14 to
1c089d9
Compare
stephen-dwq
left a comment
There was a problem hiding this comment.
lgtm; i can see no races and optimizations are correct
…uns its postprocess #857 as reviewed, squashed for the rebase onto main: its commits predate #794 (END_PARTITION) and #858 (TokenStop), and the ones that conflict are intermediate designs later commits replace. Reconciled with both: - END_PARTITION from check_stop: handled in Worker._check_stops, on both paths that call the model's check (before the runtime, or on its STOPS break): a row inside a loop fails, any other ends its partition through the request's partition_done (renamed from stream_partition_done, as #794 renamed the rest), or straight on the runtime once the request is built. - An inline route skips a step with withheld outputs, as main's _route_and_send_fast did. - Qwen3.5 declares no policies of its own: TokenStop(runtime_check=True) derives them (ARNodeSubmodule.get_stop_policies), as StopKind.EOS, since main's loop cap is the token budget; its prompt walks keep check_stop, which can end the partition there. Other TokenStop models are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
main's test called _route_and_send_fast, which this PR replaces; the same check is now the request's inline column. The log-probs case's batch stub gains the withheld_outputs the inline test reads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Most
check_stoppolicies follow one of two forms: AR models check the iter count against a maximum, plus an EOS check if ignore_eos is not set, and diffusion/flow models just do the iter count check. This PR ports both of these common checks into the Rust runtime, with the option for a submodule-level override.What and why
Declared stop policies:
NodeSubmodule.get_stop_policies(graph_walk) -> dict[str, StopPolicy, ...] | None(dict from loop name -> policy). The default isNone, which keeps calling the submodule'scheck_stop.StopKind.ARandStopKind.ITERencode the AR-style and diffusion-style stop checks described above.StopKind.EOSchecks only the token, for a loop whose cap (Model.get_loop_caps) is its budget, asmain'sTokenStopmodels' are.GraphRuntime.check_stops; the per-row decision is the policy's own (StopPolicy.stops), and each runtime keeps a request's limits (StopLimits) per (node, walk) on its request info, registered whenever the worker receives the request's forward-pass info, so they go when the request is removed. The runtime reads the policies' token column from the step's stop buffers (BatchedModelOutput.check_stop_buffers), so the fast path needs both declared policies and batched output. A step the runtime can't check (no declared policies, a step that mixes walks, or no stop buffer for the token column) still callscheck_stop.AR. ATokenStop(Performance 1/7: shared engine, conductor and component changes #858) withruntime_check=Truederives its policy (ARNodeSubmodule.get_stop_policies):EOSon its loop's walk, while itsprefill_walkskeepcheck_stop, which can end the partition there (A stop token sampled at prefill ends the reply, and max_output_tokens counts the prefill's token #794). Qwen3.5 sets it; the otherTokenStopmodels are unchanged here. Qwen3.5 and Orpheus return their token as a stop buffer. Chatterbox and Qwen3-TTS keepcheck_stop: they count their own steps inrequest_state, not by the loop's iteration count. Whisper keepscheck_stoptoo: since Whisper/Qwen3-ASR PR 5: serving performance #568 its stop check also keeps each token for the word-timestamp align walk.Postprocess as begin / resume. Inside the runtime, a step's postprocess is three phases: check stops (and stop loops), route, send. The worker drives it as
begin_postprocess(request)thenresume_postprocess(...), and the runtime stops for the worker only where a phase needs Python, named byPostprocessNeed:STOPS: the runtime's own check failed or missed a row's limits, so the model'scheck_stopdecides; resume with the stops and the rows it failed. A step the runtime doesn't check never stops here: the worker runscheck_stopbefore beginning and passes its stops in (PostprocessRequest.stopswithruntime_stops=False), and the runtime takes them as final.STORE: the outputs have to be stored as tensors before routing; resume with them. A step that returns log-probs (Sampler: return the sampled token's log-prob and ids on /generate #914) always takes this break, since the store reads each row's log-probs for the route.SEND: the send needs what only Python has (forward-pass info when a walk completes, the final publish of stopped rows, tensors to register, local streams, new-token counts, profiling); resume with theSendInput.PhasedPostprocessis the Python implementation over each runtime's phase methods; the Rust runtime runs the same flow natively (rust/src/graph/runtime/postprocess.rs). It holds the GIL, except around the route and send of an inline step, which touch nothing in Python; that time is recorded asrust.postprocess.nogil. This replacesroute_and_send_inlineand the worker's fast-path fallbacks:_postprocess_batchis now one begin / resume loop with a handler per break. Before it begins, the worker hands the engine the step's host outputs (Engine.step_finished; today that keys the step's tokens for prefix reuse), so a request that stops on this step is up to date first.Instrumentation. Under
MSTAR_PHASE_TIMING,worker.msg.<type>times each worker message type, with stage laps inside_add_new_requestand_remove_request.worker_phasesruns across this PR.worker.postprocess.runtimeis all the time inside the runtime's begin / resume calls. Each break the worker handles is timed under its own name:check_stop(the model'scheck_stop, before the runtime or at a STOPS break),store_tensorsandsend_prep.prematerializeandtailare the stages before and after. The oldcheck_stop_tail,stop_loops,route_outputs,register_outputs,routeandsendstages are gone.rust.route_and_send_inline.nogilbecomesrust.postprocess.nogil.END_PARTITION(#794). The worker takes it out of the model's stops in_check_stops, which both paths that callcheck_stopgo through: a row inside a loop fails, as onmain, and any other ends its partition, through the request'spartition_doneor, at aSTOPSbreak, on the runtime directly. A step whose stop check withheld an output (StopCheck.withheld) routes throughSTORE, never inline, asmain's fast path refused it.Docs.
docs/adding_models.rstdocumentsget_stop_policiesandTokenStop.runtime_check.Evidence
1× H200,
main9c05c56 (this PR's base) against this PR; Rust graph + ZMQ, SHM arena, prefix cache off, closed loop,benchmark/worker_phasesat period 100. ABBA, two servers per arm, 0 failed requests. Qwen3.5 0.8B as configured inconfigs/qwen3_5_0.8b.yaml(--ignore-eos, 256–512 output tokens); Orpheus withconfigs/orpheus_colocated.yaml(LLM and SNAC on one worker).Postprocess work (ms per worker iteration, both servers pooled):
worker.postprocess_batchless the waits in it, the wait for the step (completion_event_sync) and the stop check's host copy (prematerialize).maincheck_stopstage falls from 0.11–0.15 ms (Qwen3.5) and 0.15–0.31 ms (Orpheus) per iteration to 0.01–0.03 ms; what remains are the steps the runtime doesn't check (prefill and mixed steps). The runtime's own begin / resume span is 0.20–0.37 ms, and holds the route and send the worker ran before.Throughput:
mainPer-request message cost (the new
worker.msg.*spans, measured before the rebase ontomain; Qwen3.5 c=128, 2,306 requests): about 0.68 ms of main-thread CPU per request.These are follow-up targets, not changed here.
Tests
test/modular/test_stop_policies.py: each model's declared policies agree with its owncheck_stop, Qwen3.5's as derived from itsTokenStop;test/rust/test_runtime_check_stops.py: both runtimes, over the token column the worker'sPostprocessRequesthands them, through a speculated decode loop, stop what the policy does; a recycled handle has no limits until its own are registered.test/modular/test_postprocess_request.py: the worker'sPostprocessRequestroutes a combined batch under its walks' output signals, and carries the model's stops when the runtime doesn't check them.test/rust/test_postprocess.py: the Rust runtime's native begin / resume matchesPhasedPostprocessover the same runtime: every break, the frames sent and the loop counters, across runtime and model stops, inline and stored outputs, int32/int64 tokens, and host rows in or out of order.ignore_eosfails 14 of 24.Not in this PR
check_stopin Python.test/waypoint/serve_rollout.py, so lowering it needs a replacement signal there.AI assistance was used to write this change and its description.
🤖 Generated with Claude Code