Fix: capture dep_gen on the host orchestrator for host_build_graph - #1492
Conversation
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling 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 (22)
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 |
|
CI note — the first attempt of both no-hardware Not this PR: the same assertion fails on an unrelated branch — Flagging rather than waving it through: it looks like a real race in the L4→L3 |
1ba1035 to
b40dac3
Compare
|
Self-review pass (with codex + gemini cross-check) turned up findings; all fixed and re-pushed. Must fix — the new capture had zero CI coverage. All four dep_gen smoke steps hardcoded the Should fix — process-global capture state. Should fix — silent empty graph. Should fix — st covered only one of three edge sources. The shared Should fix — doc drift I introduced. Also folded in: Re-verified after the fixes: hbg sim suite 14 passed; both dep_gen st cases pass with and without One note on the earlier |
b40dac3 to
7b75109
Compare
Fixes hw-native-sys#1487 host_build_graph called dep_gen_aicpu_init() from its scheduler cold path but never called dep_gen_aicpu_set_orch_thread_idx() or _flush(), leaving the subsystem initialised and half-wired. It cannot be finished the tensormap_and_ringbuffer way: host_build_graph orchestrates on the host, so there is no device orchestrator thread to own a ready queue, nothing on the device to flush, and the AICPU never submits a task. Capture the graph where this runtime actually builds it. compute_task_fanin takes an Annotate hook that fires alongside its existing emit, so creator retention and tensormap lookups are recorded as the runtime resolves them, and submit_task_common opens the task entry and records declared dependencies. What lands in deps.json is therefore the runtime's own dependency resolution rather than a replay's reconstruction of it, and it cannot drift from compute_task_fanin semantics. No ring, no collector, no reconcile: the orchestration completes before any scheduler thread starts, so nothing can be dropped under back-pressure. The capture state is thread-local, matching the per-thread runner both c_api_shared.cpp files already key off a pthread_key_t — two runners on two threads build two independent graphs, the same per-runner isolation the device-orch shape gets from DeviceRunner::dep_gen_collector_ being a member. emit() refuses to write when no capture ran on its thread, so a mis-wired arm reports instead of producing an empty-but-valid deps.json. deps.json keeps the schema the device-orch replay emits, so deps_viewer and the swimlane join read both runtimes identically. Two st cases cover it: one runs the same vector_example orchestration as the tensormap_and_ringbuffer dep_gen test and asserts the same 6 edges (which is what would catch the two shapes diverging), the other runs predicated_dispatch for the tensormap and explicit edge sources and their producer-side slice annotation. Both are wired into the a2a3 dep_gen smoke steps, sim and onboard — the default st sweep passes no --enable-dep-gen, so a capture regression is only visible there. The runner picks the shape via dep_gen_host_graph_active() and skips device collector init/start/reconcile for the host-orch one; host_build_graph's copy of dep_gen_replay.{h,cpp} is removed, matching what the device runners' weak stubs already claimed. Also guard the shared AICPU writer: a record arriving before dep_gen_aicpu_set_orch_thread_idx() has no ready queue to reach, so it is now charged to dropped_record_count instead of filling a buffer nothing can publish and surfacing later as a misleading "ready_queue full" error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixes #1487
What
host_build_graphcalleddep_gen_aicpu_init()from its scheduler cold path but never calleddep_gen_aicpu_set_orch_thread_idx()ordep_gen_aicpu_flush()— the subsystem was initialised and then left half-wired.It cannot be finished the
tensormap_and_ringbufferway: hbg orchestrates on the host (run_host_orchestrationdlsym's and runs the whole orchestration before any scheduler thread starts; the AICPU boot thread only attaches the prebuilt arena), so there is no device orchestrator thread to own a ready queue and nothing device-side to flush.So this PR gives hbg dep_gen for real, in the shape its architecture allows: capture the graph as the host orchestrator builds it.
compute_task_fanintakes anAnnotatehook that fires alongside its existingemit, so creator-retention and tensormap-lookup producers are recorded as the runtime resolves them;submit_task_commonopens the task entry and records declared dependencies. The graph is the runtime's own answer, not a replay's reconstruction, so it cannot drift fromcompute_task_faninsemantics.dep_gen_host_graph_active()and skips device collector init/start/reconcile (and the device DFX flag) for the host-orch runtime.deps.jsonkeeps the exact schema the device-orch replay emits, sodeps_viewerand the swimlane join read both runtimes identically.dep_gen_replay.{h,cpp}is removed — which is what the device runners' weak stubs already claimed ("host_build_graph has no replay implementation today").Separately, the shared AICPU writer now guards the negative orchestrator index: a record arriving before
dep_gen_aicpu_set_orch_thread_idx()has no ready queue to reach, so it is charged todropped_record_countinstead of filling a buffer nothing can publish and surfacing later as a misleading "ready_queue full" error. Note thequeues[-1]write the issue predicted was already impossible —DeviceProfilerEngine::enqueue_ready→wait_for_ready_queue_spacerejects a negative index beforewrite_ready_entry.Equivalence check
The new st case runs the same
vector_exampleorchestration thetensormap_and_ringbufferdep_gen test uses, and asserts the same 6 edges. Cross-checking the two artifacts directly (topology by submit order, since hbg keeps the inner manual scope on ring 0 where trb moves it to ring 1):Testing
-LE requires_hardware)test_dep_gen_collector_aicpu(written failing first)tests/ut/py)tests/st/a2a3/host_build_graph--enable-dep-gen--enable-dep-gentask-submit) — hbg dep_gen + vector_example with--enable-dep-gen--enable-dep-gentests/st/a2a3/host_build_graphThe full
--runtime tensormap_and_ringbuffersim sweep shows 7 failures / 25 errors on this box (a5 cases collected undera2a3sim, multi-device L3 cases against a 1-device sim pool). A baseline worktree atupstream/mainwithout this change reproduces the identical set (7 failed, 66 passed, 2 skipped, 25 errors), so they are pre-existing environment limits, not regressions.🤖 Generated with Claude Code