Doc: document swimlane / dep_gen split workflow for perf-sensitive runs#954
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds documentation explaining how dependency arrows are generated in L2 swimlane profiling. It introduces a reference note in ChangesDependency Generation and Swimlane Profiling Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for L2 swimlane profiling and dependency generation, adding a new section that explains how dependency arrows are integrated into swimlane records. It describes both the default paired-flag workflow and the split workflow recommended for performance-sensitive measurements. There are no review comments, and I have no feedback to provide.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/l2-swimlane-profiling.md`:
- Around line 220-296: The doc is inconsistent: dependency arrows come from
deps.json produced by dep_gen and joined by swimlane_converter (as described in
this section), but earlier text still claims arrows follow fanout[]; update all
earlier references (mentions of fanout[], any statements in the intro or the
L2SwimlaneAicpuTaskRecord comment) to state that swimlane records carry timing
only and that visual arrows are derived from deps.json/dep_gen and merged by
swimlane_converter (and optionally via --deps-json), clarifying that fanout[] is
not embedded per-task in l2_swimlane_records.json but instead is represented in
deps.json.
🪄 Autofix (Beta)
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
Run ID: f712e7d7-2db6-4bcb-a02e-2b8cd7c8366a
📒 Files selected for processing (2)
docs/dfx/dep_gen.mddocs/dfx/l2-swimlane-profiling.md
a8e3e9b to
2fb7123
Compare
The code already supports running --enable-dep-gen and --enable-l2-swimlane in separate launches (swimlane_converter.load_deps_json docstring describes this as "the typical workflow"), but the user-facing docs/dfx/l2-swimlane-profiling.md doesn't mention dep_gen anywhere. Result: users wonder why their Perfetto trace has no fanout arrows, miss the converter's "no deps.json" hint, or pair both flags even when they're doing strict overhead measurement. Changes: - l2-swimlane-profiling.md §3.5 "Dependency arrows from dep_gen" — new subsection between §3.4 and §4. Explains why the device hot path omits fanout (PR hw-native-sys#863), shows the two artifacts and the converter join, then presents the two workflows side by side (paired = default; split = perf measurement) with the criteria for choosing. Calls out what you don't need to script (input shape coupling, per-run dep_gen). - dep_gen.md §3 — cross-link forward to the split-workflow subsection so someone arriving at dep_gen first sees the alternative. No code changes — the workflow is already supported end-to-end. Test plan: - pre-commit clean (markdownlint compact + aligned_delimiter table satisfied) - intra-doc link `l2-swimlane-profiling.md#35-dependency-arrows-from-dep_gen` matches the heading slug GitHub will generate
Why
The two-launch workflow (one
--enable-dep-gencapture per topology + N--enable-l2-swimlaneruns, joined byswimlane_converter --deps-json) is already supported end-to-end —swimlane_converter.load_deps_jsondocstring even calls it "the typical workflow". But the user-facingdocs/dfx/l2-swimlane-profiling.mddoesn't mention dep_gen anywhere. Result:Confirmed against
paged_attention_unrollCase1 in the previous session: swimlane level 4 + dep_gen combined adds < 10 µs per round on onboard a2a3; for default usage paired is fine, the split is for the strict-perf case only.What changes
docs/dfx/l2-swimlane-profiling.md§3.5 "Dependency arrows from dep_gen" — new subsection between §3.4 and §4. Covers:--deps-jsonflag and the missing-deps.jsonhint message.docs/dfx/dep_gen.md§3 — cross-link forward to the split-workflow subsection so someone arriving at dep_gen first sees the alternative.What doesn't change
No code. The workflow is already supported and the converter already prints the right discovery breadcrumb when
deps.jsonis missing — the doc is the only gap.Test plan
l2-swimlane-profiling.md#35-dependency-arrows-from-dep_genmatches GitHub's heading-slug derivation