Update: improve L2 swimlane SPMD flow anchors#1300
Conversation
indigo1973
commented
Jul 8, 2026
indigo1973
commented
- Collapse SPMD dependency anchors by func_id and task_id using the earliest-start subtask so MIX AIC/AIV tasks stay distinct.
- Fold AIC/AIV setup preview into the Worker View task bar while keeping kernel and setup durations in hover args.
- Render resolve on a sibling scheduler lane and anchor complete flows on the same representative SPMD task slices as dependency flows.
- Update the L2 swimlane documentation to describe the new lane and anchor semantics.
|
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 updates the Perfetto swimlane converter's Worker View task bar timing to start at ChangesSwimlane profiling rendering and anchoring changes
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
simpler_setup/tools/swimlane_converter.py (2)
1297-1298: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate stale SPMD anchoring comments and verbose message.
The
_dependency_flow_anchor_rowsfunction now collapses byfunc_idwith earliest-start selection, but two comments still describe the old "min core_id per core_type" approach:
- Line 1297–1298:
# SPMD logical tasks anchor dependency arrows on the min-core_id subtask / # row per core_type (AIC and AIV separately when both are present).- Line 1355:
"dependency arrows anchor on min core_id subtask per core_type"These contradict the new implementation and the updated documentation in
l2-swimlane-profiling.md(lines 475–482, 526–527).📝 Proposed fix
# SPMD logical tasks anchor dependency arrows on the earliest-start - # subtask row per core_type (AIC and AIV separately when both are present). + # subtask row per (func_id, task_id) group, so MIX tasks with shared + # task_id but distinct AIC/AIV func_id values remain distinct. flow_id = 0print( f" SPMD tasks: {len(spmd_task_ids)} logical task(s); " - "dependency arrows anchor on min core_id subtask per core_type" + "dependency arrows anchor on earliest-start subtask per (func_id, task_id)" )Also applies to: 1355-1355
🤖 Prompt for 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. In `@simpler_setup/tools/swimlane_converter.py` around lines 1297 - 1298, Update the stale SPMD anchoring comments and the verbose message in _dependency_flow_anchor_rows to match the current func_id-based earliest-start behavior, replacing the old “min-core_id per core_type” wording in the nearby comment block and the dependency-arrow anchor message so they no longer describe the outdated AIC/AIV-specific logic.
1390-1390: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate stale resolve phase comment.
Line 1390 still says
# Inner phase — nests inside Complete or Dummy via time containment, but resolve is now rendered on a sibling scheduler sub-lane (lane 1 viasched_lane_tid(thread_idx, 1)), not nested via time containment. This contradicts the updated documentation atl2-swimlane-profiling.mdlines 235–245 which states resolve is rendered on a siblingSched_Ntid.📝 Proposed fix
- # Inner phase — nests inside Complete or Dummy via time containment + # Inner phase — rendered on a sibling Sched_N sub-lane (adjacent tid) "resolve": "vsync_highlight_color", # on_task_complete: walk consumer list🤖 Prompt for 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. In `@simpler_setup/tools/swimlane_converter.py` at line 1390, The comment in swimlane_converter.py is stale and still describes resolve as an “Inner phase” nested by time containment, but the current logic renders resolve on a sibling scheduler sub-lane. Update the comment near the resolve-related code in the swimlane conversion flow to match the actual behavior and terminology used by sched_lane_tid(thread_idx, 1) and the sibling Sched_N lane documented in l2-swimlane-profiling.md; keep the wording aligned with the resolved lane layout rather than nesting semantics.
🧹 Nitpick comments (1)
simpler_setup/tools/swimlane_converter.py (1)
1927-1927: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider consolidating the duplicate flow epsilon constants.
flow_epsilon(line 1303) andFLOW_EPSILON_US(line 1927) are both0.01but defined separately. Consolidating into a single module-level constant would prevent silent divergence if one is changed without the other.Also applies to: 1303-1303
🤖 Prompt for 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. In `@simpler_setup/tools/swimlane_converter.py` at line 1927, Duplicate flow epsilon values are defined separately as flow_epsilon and FLOW_EPSILON_US; consolidate them into a single module-level constant in swimlane_converter.py. Update the code near the existing flow_epsilon definition and the FLOW_EPSILON_US usage so both paths reference the same symbol, keeping one source of truth for the epsilon value. Ensure any downstream references in the related flow-conversion logic continue to use the shared constant.
🤖 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.
Outside diff comments:
In `@simpler_setup/tools/swimlane_converter.py`:
- Around line 1297-1298: Update the stale SPMD anchoring comments and the
verbose message in _dependency_flow_anchor_rows to match the current
func_id-based earliest-start behavior, replacing the old “min-core_id per
core_type” wording in the nearby comment block and the dependency-arrow anchor
message so they no longer describe the outdated AIC/AIV-specific logic.
- Line 1390: The comment in swimlane_converter.py is stale and still describes
resolve as an “Inner phase” nested by time containment, but the current logic
renders resolve on a sibling scheduler sub-lane. Update the comment near the
resolve-related code in the swimlane conversion flow to match the actual
behavior and terminology used by sched_lane_tid(thread_idx, 1) and the sibling
Sched_N lane documented in l2-swimlane-profiling.md; keep the wording aligned
with the resolved lane layout rather than nesting semantics.
---
Nitpick comments:
In `@simpler_setup/tools/swimlane_converter.py`:
- Line 1927: Duplicate flow epsilon values are defined separately as
flow_epsilon and FLOW_EPSILON_US; consolidate them into a single module-level
constant in swimlane_converter.py. Update the code near the existing
flow_epsilon definition and the FLOW_EPSILON_US usage so both paths reference
the same symbol, keeping one source of truth for the epsilon value. Ensure any
downstream references in the related flow-conversion logic continue to use the
shared constant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f6450c5-5586-486e-996d-6d3c6280f47b
📒 Files selected for processing (2)
docs/dfx/l2-swimlane-profiling.mdsimpler_setup/tools/swimlane_converter.py
- Collapse SPMD dependency anchors by func_id and task_id using the earliest-start subtask so MIX AIC/AIV tasks stay distinct. - Fold AIC/AIV setup preview into the Worker View task bar while keeping kernel and setup durations in hover args. - Render resolve on a sibling scheduler lane and anchor complete flows on the same representative SPMD task slices as dependency flows. - Update the L2 swimlane documentation to describe the new lane and anchor semantics.
1 participant
