Refactor dep gen and l2 swimlane collectors to common#1262
Refactor dep gen and l2 swimlane collectors to common#1262vegetabledoww wants to merge 1 commit into
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 consolidates architecture-specific L2SwimlaneCollector and DepGenCollector implementations from src/a2a3 and src/a5 into shared src/common headers/sources, removes the now-redundant platform-specific files, updates memory-copy calls to non-SVM-aware helpers, reworks buffer pool seeding logic, rewires CMake build sources, and updates documentation links accordingly. ChangesCollector consolidation into src/common
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request consolidates the platform-specific implementations of dep_gen_collector and l2_swimlane_collector from src/a2a3 and src/a5 into a unified common implementation under src/common/platform. It updates build configurations, documentation, and unit tests to point to the new shared paths. Additionally, it generalizes platform-specific terminology to generic categories (e.g., "non-SVM platforms") and optimizes the initialization of L2SwimlaneCollector to seed the device-side free queue with multiple buffers. No review comments were provided, so there is no further feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
vegetabledoww
force-pushed
the
fix/issue-1253-common-collectors-v2
branch
from
3c2728a to
c229fdc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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`:
- Line 711: The collector links in the profiling doc are resolving to the wrong
location because the relative path is missing one directory level; update the
`L2SwimlaneCollector` link targets in both the a2a3 and a5 sections to use the
shared header path with the correct `../../src/...` prefix so the markdown
points to the actual file.
🪄 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: 21b3d8c8-3a69-46bf-9dcc-12da23a8d58e
📒 Files selected for processing (21)
docs/dfx/dep_gen.mddocs/dfx/l2-swimlane-profiling.mddocs/hardware/cache-coherency.mddocs/profiling-framework.mdsrc/a2a3/platform/include/host/dep_gen_collector.hsrc/a2a3/platform/include/host/l2_swimlane_collector.hsrc/a2a3/platform/onboard/host/CMakeLists.txtsrc/a2a3/platform/shared/host/dep_gen_collector.cppsrc/a2a3/platform/shared/host/l2_swimlane_collector.cppsrc/a2a3/platform/sim/host/CMakeLists.txtsrc/a5/platform/onboard/host/CMakeLists.txtsrc/a5/platform/shared/aicpu/dep_gen_collector_aicpu.cppsrc/a5/platform/shared/aicpu/l2_swimlane_collector_aicpu.cppsrc/a5/platform/sim/host/CMakeLists.txtsrc/common/platform/include/host/dep_gen_collector.hsrc/common/platform/include/host/l2_swimlane_collector.hsrc/common/platform/shared/aicpu/dep_gen_collector_aicpu.cppsrc/common/platform/shared/aicpu/l2_swimlane_collector_aicpu.cppsrc/common/platform/shared/host/dep_gen_collector.cppsrc/common/platform/shared/host/l2_swimlane_collector.cpptests/ut/cpp/CMakeLists.txt
💤 Files with no reviewable changes (6)
- src/a2a3/platform/shared/host/l2_swimlane_collector.cpp
- src/a5/platform/shared/aicpu/dep_gen_collector_aicpu.cpp
- src/a2a3/platform/shared/host/dep_gen_collector.cpp
- src/a5/platform/shared/aicpu/l2_swimlane_collector_aicpu.cpp
- src/a2a3/platform/include/host/dep_gen_collector.h
- src/a2a3/platform/include/host/l2_swimlane_collector.h
| ``` | ||
|
|
||
| [`L2SwimlaneCollector`](../src/a2a3/platform/include/host/l2_swimlane_collector.h) | ||
| [`L2SwimlaneCollector`](../src/common/platform/include/host/l2_swimlane_collector.h) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the collector link targets.
../src/... from docs/dfx/l2-swimlane-profiling.md resolves under docs/src/..., so both links are broken as written. Use ../../src/... for the shared header in both sections.
Suggested fix
-[`L2SwimlaneCollector`](../src/common/platform/include/host/l2_swimlane_collector.h)
+[`L2SwimlaneCollector`](../../src/common/platform/include/host/l2_swimlane_collector.h)Apply the same replacement in both the a2a3 and a5 sections.
Also applies to: 841-841
🤖 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 `@docs/dfx/l2-swimlane-profiling.md` at line 711, The collector links in the
profiling doc are resolving to the wrong location because the relative path is
missing one directory level; update the `L2SwimlaneCollector` link targets in
both the a2a3 and a5 sections to use the shared header path with the correct
`../../src/...` prefix so the markdown points to the actual file.
ca72843 to
16246e8
Compare
Hoist the shared DepGen and L2 Swimlane collector implementations from the per-architecture a2a3/a5 platform trees into src/common/platform. Update a2a3/a5 onboard and sim build lists to compile the common host collectors, and keep the architecture-specific memory transport behind each platform's profiling_copy.cpp callbacks. Preserve the PR hw-native-sys#1162 buffer/drop-reduction behavior after relocation, including per-AICPU-thread drain/collector sharding and initial free_queue seeding up to the slot count. Also refresh related docs and stale DepGen comments to describe the current in-memory replay path instead of a submit_trace.bin intermediary. Validation: C++ UT build passed; no_hardware ctest passed 40/40; pip editable install passed; a2a3 onboard DepGen and L2 Swimlane DFX tests passed 4/4 via task-submit.
vegetabledoww
force-pushed
the
fix/issue-1253-common-collectors-v2
branch
from
16246e8 to
dd4c487
Compare
1 participant
Summary
Closes #1253.
This PR hoists the shared DepGen and L2 Swimlane collectors out of the per-architecture a2a3/a5 platform trees into
src/common/platform.profiling_copy.cppimplementation.Validation
g++ -fsyntax-onlyfor the common host collectors with both a2a3 and a5 include paths.cmake --build /data/jinzongquan/simpler/build/ut_cpp_issue1253 -j8ctest --test-dir /data/jinzongquan/simpler/build/ut_cpp_issue1253 --output-on-failure -L no_hardwarepip install --no-build-isolation -e .task-submit:test_dep_gen.pytest_dep_gen_chain.pytest_l2_swimlane.pytest_l2_swimlane_mixed.py--platform a2a3 --enable-l2-swimlane 4 --enable-dep-gen.Notes
--enable-l2-swimlane 1was not used for the final onboard validation because level 1 only captures AICore timing and does not include the AICPU/phase data required by the sched overhead analysis.