Skip to content

refactor(dfx): unify L2 phase buffer switching#1391

Merged
ChaoWao merged 1 commit into
hw-native-sys:mainfrom
vegetabledoww:issue-1385-unify-phase-buffer-switch
Jul 18, 2026
Merged

refactor(dfx): unify L2 phase buffer switching#1391
ChaoWao merged 1 commit into
hw-native-sys:mainfrom
vegetabledoww:issue-1385-unify-phase-buffer-switch

Conversation

@vegetabledoww

@vegetabledoww vegetabledoww commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reuse DeviceProfilerEngine for sched/orch phase buffer switching and recovery.
  • Centralize free-queue ownership, head updates, and sequence bookkeeping.
  • Keep phase startup acquisition non-blocking through try_pop_free().
  • Clarify task/phase/AICore wrapper naming.
  • Add unit tests for buffer switching, replacement failure, recovery, and empty startup queues.
  • Preserve the reserved sequence number when recovering after a replacement-pop failure.

Fixes #1385

Validation

  • test_profiler_device_engine: passed
  • a2a3 AICPU kernel: built successfully
  • a5 AICPU kernel: built successfully
  • pre-commit checks: passed

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5a752f7d-2e49-4764-a335-6ef290fc8db2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

DeviceProfilerEngine now centralizes free-buffer claiming and adds non-blocking startup acquisition. The L2 swimlane collector uses typed contexts and engine-based phase-buffer operations, with new unit tests covering queue transitions and empty-queue behavior.

Changes

Device profiler buffer flow

Layer / File(s) Summary
Free-buffer claiming and acquisition
src/common/platform/include/aicpu/profiler_device_engine.h
Free-slot ownership transfer is centralized in claim_free; try_pop_free adds non-blocking acquisition, while pop_free retains blocking behavior.
L2 swimlane buffer integration
src/common/platform/shared/aicpu/l2_swimlane_collector_aicpu.cpp
Typed device-module contexts route phase priming, recovery, switching, and pending AICore publication through DeviceProfilerEngine.
Engine behavior tests and build wiring
tests/ut/cpp/CMakeLists.txt, tests/ut/cpp/common/test_profiler_device_engine.cpp
Adds the unit-test target, fake module, and coverage for switching, replacement failure, free pops, and empty startup queues.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Collector as L2SwimlaneCollector
  participant Engine as DeviceProfilerEngine
  participant FreeQueue
  participant ReadyQueue
  Collector->>Engine: prime or recover phase buffer
  Engine->>FreeQueue: claim free slot
  Collector->>Engine: switch full buffer
  Engine->>ReadyQueue: enqueue completed buffer
  Engine->>FreeQueue: pop replacement buffer
Loading

Possibly related PRs

Poem

A rabbit found a free-slot ring,
And taught the buffers how to spring.
“Try” hops lightly, “pop” waits true,
Phase lanes bloom in sched and orch too.
Tests twitch their noses: all paths go through!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.62% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No meaningful pull request description was provided to assess against the code changes. Add a short description of the change and its purpose so reviewers can verify it matches the patch.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly matches the main refactor to unify L2 phase buffer switching.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the DeviceProfilerEngine to extract buffer claiming logic and introduces a non-blocking try_pop_free method. It also templatizes and refactors the L2 swimlane collector to use this unified engine for buffer management, switching, and recovery, replacing manual queue operations. Additionally, unit tests are added for the profiler device engine. A bug was identified in acquire_phase_slot where recovering a phase buffer after a switch failure causes a sequence number gap because state->head.current_buf_seq + 1 is passed instead of state->head.current_buf_seq.

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.

Comment thread src/common/platform/shared/aicpu/l2_swimlane_collector_aicpu.cpp

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@src/common/platform/shared/aicpu/l2_swimlane_collector_aicpu.cpp`:
- Around line 887-890: The recovery path in L2SwimlaneDeviceModule should reuse
the current buffer sequence instead of advancing it: update pop_free() in
src/common/platform/shared/aicpu/l2_swimlane_collector_aicpu.cpp:887-890 to pass
state->head.current_buf_seq. Update the corresponding test in
tests/ut/cpp/common/test_profiler_device_engine.cpp:138-154 to pass the existing
sequence and assert that it remains 8.
🪄 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: 10ec047d-7f7f-4bc9-b367-921432992124

📥 Commits

Reviewing files that changed from the base of the PR and between 6e910df and 936b457.

📒 Files selected for processing (4)
  • src/common/platform/include/aicpu/profiler_device_engine.h
  • src/common/platform/shared/aicpu/l2_swimlane_collector_aicpu.cpp
  • tests/ut/cpp/CMakeLists.txt
  • tests/ut/cpp/common/test_profiler_device_engine.cpp

Comment thread src/common/platform/shared/aicpu/l2_swimlane_collector_aicpu.cpp
@vegetabledoww
vegetabledoww force-pushed the issue-1385-unify-phase-buffer-switch branch from 936b457 to 73e8ccd Compare July 17, 2026 08:47
Route sched and orch phase buffer rotation through DeviceProfilerEngine so free-queue ownership, head updates, and sequence bookkeeping have a single implementation. Keep startup acquisition non-blocking and add engine-level tests for switching and recovery.

Refs hw-native-sys#1385
@ChaoWao
ChaoWao force-pushed the issue-1385-unify-phase-buffer-switch branch from 73e8ccd to 8a58186 Compare July 18, 2026 00:41
@ChaoWao
ChaoWao merged commit b14829e into hw-native-sys:main Jul 18, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Code Health] Unify L2 swimlane buffer-switch wrappers: dedup phase free-queue pop onto engine helpers

2 participants