Repository navigation
Support: trace native decode overlap - #1811
Merged
ChaoWao merged 1 commit intoAug 17, 2026
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change adds host-clock tracing for native pipeline lifecycle phases. It records dispatch identities and validates adjacent-dispatch preparation overlap, launch ordering, and ordered release through ChangesNative pipeline tracing and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Orchestrator
participant WorkerThread_submit_dispatch
participant NativeRunContext
participant strace_timing
Orchestrator->>Orchestrator: Emit graph_build and retirement spans
WorkerThread_submit_dispatch->>WorkerThread_submit_dispatch: Emit frame_staging span
NativeRunContext->>NativeRunContext: Record device trace start
NativeRunContext->>strace_timing: Provide native pipeline spans
strace_timing->>strace_timing: Validate adjacent dispatch overlap and release order
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/dfx/host-trace.md`:
- Around line 122-125: Update the adjacent-dispatch overlap requirements in the
command documentation to state that native_prepare(N+1) must finish after
device_execution(N) starts and no later than device_execution(N) ends, while
preserving the existing start-before-end and ordered_release constraints.
In `@src/common/hierarchical/pipeline_trace.h`:
- Around line 49-53: Update the STRACE formatting call in the pipeline trace
emission to populate the tid field with the actual native thread ID, matching
the mechanism used by strace_tid(), while retaining getpid() for pid and leaving
the remaining fields unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 68aad3ec-83ea-4ed3-9b56-9b546304d1ca
📒 Files selected for processing (11)
docs/dfx/host-trace.mdsimpler_setup/tools/strace_timing.pysrc/common/hierarchical/orchestrator.cppsrc/common/hierarchical/pipeline_trace.hsrc/common/hierarchical/types.hsrc/common/hierarchical/worker_manager.cppsrc/common/log/include/common/strace.hsrc/common/platform/onboard/host/c_api_shared.cppsrc/common/worker/native_run_context.htests/st/a2a3/host_build_graph/concurrent_prepare_stress/test_concurrent_prepare_stress.pytests/ut/py/test_strace_timing.py
Crane-Liu
force-pushed
the
codex/deepseek-mtp-runtime-trace
branch
from
August 13, 2026 01:56
32ed069 to
415760e
Compare
This was referenced Aug 13, 2026
ChaoWao
force-pushed
the
codex/deepseek-mtp-runtime-trace
branch
5 times, most recently
from
August 17, 2026 06:58
08eef95 to
7fc6166
Compare
The phased native lane claims run N+1's preparation runs concurrently with
run N's device execution. Nothing checked it, so make it machine-checkable
from a captured log:
python -m simpler_setup.tools.strace_timing <log> --assert-native-overlap
Per adjacent run on one child process the command requires `bind(N+1)` to
overlap `runner_run(N)` — the intervals intersect — and `runner_run(N+1)`
not to start before `claim_release(N)`. The A2/A3 concurrent-prepare
stress test asserts it over all 39 adjacent pairs without changing
scheduling behavior.
Both windows already exist in the `simpler_run` tree, and the root span
already carries `run_id / dispatch_id / run_epoch / slot_id / generation`;
every sub-span of a run shares its `(pid, inv)`, so grouping by invocation
is what joins the windows to the identity. Reading `bind` rather than the
whole prepare is conservative: `bind` sits inside prepare, so an overlap it
reports is one the prepare certainly had.
`simpler_run.claim_release` is the one span this needed. It wraps
`release_native_run` inside finalize, the point a successor's launch becomes
admissible, and no other span marks that boundary. `l3.post_fence_retirement`
covers the L3 orchestrator's `release_run` tail for the same reason, and
`l3.dispatch` / `l3.frame_submit` gain the dispatch's pipeline lease so the
L3 side is lease-aware. Both new spans join a family that already exists, so
no view has to learn to filter them and neither can displace an invocation
root the way a second depth-0 span would.
`run_epoch` is the direct-chip lane's only identity — `_submit_chip_run_direct`
defaults `run_id` and `dispatch_id` to zero — so ordering falls back to it.
`NativeDispatchIdentity.sequence` makes that choice in the parser, where it is
visible, rather than letting one record field carry two meanings.
A K-deep pipeline is not K threads: the direct-chip lane drives prepare(N+1)
and finalize(N) from one OS thread, so a 40-run K=2 stress puts 40 overlapping
run lifetimes on one tid. Perfetto nests slices by timestamp containment within
a track, so `--swimlane` was rendering run N+1's spans inside run N's root —
false nesting that hid the overlap the view exists to show. A thread whose
depth-0 spans overlap is now split into one lane per pipeline slot, which reads
as a sequence because a run holds its slot exclusively; the overlap shows across
lanes instead. Each slice keeps `os_tid`, and a thread that ran sequentially —
the L3 scheduler, which carries a slot but never interleaves — keeps its tid.
`--require-hidden-prepare` adds the stronger claim that preparation also
finishes inside the predecessor's device window. That is a statement about
pipeline depth and is sensitive to how much host CPU a shared machine gives
the preparing thread, so it is opt-in and the scene test does not use it.
Co-authored-by: Chao Wang <26245345+ChaoWao@users.noreply.github.com>
ChaoWao
added a commit
that referenced
this pull request
Aug 17, 2026
`test_sim_device_log` installs a pipe over stderr and reads it only after every writer has finished. A pipe holds a bounded amount of unread data — 16 KiB on macOS, 64 KiB on Linux — and the two tests emit 19.3 KiB and 22.6 KiB. Wherever the total exceeds the capacity the writers block in write(2) while the parent waits for them in join/waitpid, and nothing ever drains: a deadlock, not a slow test. That is what killed `ut (macos-latest, 3.10)` on #1811 — the job ran to its 20-minute limit with 98 of 99 tests reported and no failure named. The test and the sim logger were unchanged from the head where the same job passed in 5m23s, so this is latent rather than a regression: it fires when the kernel grants the 16 KiB pipe instead of a 64 KiB one. Start the reader when the pipe is installed, so a writer never blocks however much it emits. The reader holds the buffer through a shared_ptr, so returning `Capture` by value cannot leave it writing into a moved-from string. Forking alongside the thread is safe because the record path allocates nothing — it formats into a stack buffer and calls write(2) — so a child cannot deadlock on a malloc lock the reader happened to hold. Add a regression test that shrinks the pipe to one page with F_SETPIPE_SZ and emits an order of magnitude more than fits, which reproduces the condition on Linux too; without the concurrent reader it deadlocks where the existing two tests pass. Because a deadlock cannot fail a test on its own, also bound the no-hardware ctest run with `--timeout 300`: a future regression then reports `test_sim_device_log (Timeout)` in seconds instead of consuming the job. The file's atomicity comment named Linux's PIPE_BUF. The portable floor is _POSIX_PIPE_BUF (512), which is what macOS uses; these records are ~30 bytes and inside either, so the claim holds — state the bound that actually applies.
This was referenced Aug 17, 2026
This was referenced Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
run_id / dispatch_id / slot_id / generationthrough the trace and add--assert-native-overlapfor machine-verifiable orderingTesting
pre-commit run --files ...pytest -q tests/ut/py/test_strace_timing.py(18 passed)_task_interfacebuildtest_orchestrator(54 passed)test_scheduler(66 passed)test_chip_run_lane(17 passed)task-submitbuild_package_a2a3with PTO-ISA pin0cefc9a5a1c24c62655cc345d408559595a8af32test_concurrent_prepare_overlapviatask-submit(1 passed, 40 runs / 39 adjacent pairs)