host_build_graph: capture the outer GRAPH task in the dependency graph - #1908
host_build_graph: capture the outer GRAPH task in the dependency graph#1908ChaoWao wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 27 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
06abcab to
084a70b
Compare
dep_gen opens a task's entry inside submit_task_common, and an outer GRAPH task is emitted by graph_submit_definition, which builds its slot directly and never goes through there. So no GRAPH task and no edge touching one reached deps.json: on examples/a2a3/host_build_graph/qwen3_14b_decode a 40-layer decode was described by its handful of non-Graph tasks alone, while the capture point calls itself "the sole source of truth for fanout". graph_submit_definition now opens an entry for the task it emits and instantiates compute_task_fanin with the annotating form, so the boundary edges attach to it. The DepGraphAnnotate that raises those edges moves to file scope and is shared with the ordinary path rather than duplicated. An outer GRAPH task dispatches no kernel of its own, so it records INVALID_KERNEL_ID in all three slots, and the sub-DAG it replays owns no ring slots — what is captured is its boundary, which the comment states so the coverage claim is not over-read again. On tests/st/a2a3/host_build_graph/graph_execution with --enable-dep-gen, both cases gain their Graph structure, and one of them gains an artifact it never produced: record_then_replay_2d 6 tasks / 8 edges -> 8 tasks / 10 edges record_then_replay_mix_spmd no deps.json -> 3 tasks / 0 edges The mix_spmd case submits nothing but Graphs, so dep_gen had no task to open an entry for and wrote no file at all. The 2d case gains its two outer GRAPH tasks and the two edges into them. Its count of tasks whose three kernel ids are all invalid goes 1 -> 3: the pre-existing dummy fence, plus the two GRAPH tasks, which share that signature — so an all-invalid triple marks "dispatches no kernel", not "is a Graph". Capture stays gated on dep_gen_host_graph_enabled(), whose weak fallback returns false, so a normal run adds one call per GRAPH submit — the same call the ordinary path already makes per task. Measured on qwen3-14b decode, twelve rounds after against six before: graph_submit min 0.106 -> 0.110 ms, while counters this does not touch move -0.016 (arena_h2d) to +0.007 (graph_upload) over the same interval, and the sum of the scope's minima is flat at 0.660 -> 0.661 ms. Below the floor.
|
Closing in favour of #1911, which landed the same change 27 minutes later as a strict The measurement evidence from here is now on #1911 as a review comment — the before/after One CI note that outlives this PR: this branch hit a single |
1 participant
Summary
dep_genopens a task's entry insubmit_task_common, but an outerGRAPHtask isemitted by
graph_submit_definition, which builds its slot directly and never goesthrough there — so no GRAPH task, and no edge touching one, reached
deps.jsongraph_submit_definitionnow opens an entry for the task it emits and instantiatescompute_task_faninwith the annotating form, so the boundary edges attach to itDepGraphAnnotatethat raises those edges moves to file scope and is shared withthe ordinary path instead of being duplicated
and the sub-DAG it replays owns no ring slots, which the comment now states so the
coverage claim is not over-read again
Before this,
examples/a2a3/host_build_graph/qwen3_14b_decodedescribed a 40-layerdecode by its handful of non-Graph tasks alone, while the capture point calls itself
"the sole source of truth for fanout".
Testing
tests/st/a2a3/host_build_graph/graph_executionwith--enable-dep-gen, ona2a3sim,before and after:
record_then_replay_2drecord_then_replay_mix_spmddeps.jsonwrittenmix_spmdsubmits nothing but Graphs, sodep_genhad no task to open an entry for andproduced no file at all. The 2d case gains its two outer GRAPH tasks and the two edges
into them; its count of tasks whose three kernel ids are all invalid goes 1 → 3 — the
pre-existing dummy fence plus the two GRAPH tasks, which share that signature, so an
all-invalid triple marks "dispatches no kernel", not "is a Graph".
tests/st/a2a3/host_build_graphona2a3sim(9 passed, 7 skipped),tests/st/a5/host_build_graphona5sim(5 passed)qwen3_14b_decodeon a2a3 undertask-submitCapture stays gated on
dep_gen_host_graph_enabled(), whose weak fallback returnsfalse, so a normal run adds one call per GRAPH submit — the same call the ordinary path
already makes per task. Measured on qwen3-14b decode, twelve rounds after against six
before:
graph_submitmin 0.106 → 0.110 ms, while counters this does not touch move−0.016 (
arena_h2d) to +0.007 (graph_upload) over the same interval, and the sum ofthe scope's minima is flat at 0.660 → 0.661 ms. Below the measurement floor on a shared
box.