Skip to content

Fix: skip unused task-timing tail readback - #1707

Merged
ChaoZheng109 merged 1 commit into
hw-native-sys:mainfrom
zmnobug:fix/issue-1642-skip-unused-task-timing-tail
Aug 6, 2026
Merged

ChaoZheng109 merged 1 commit into
hw-native-sys:mainfrom
zmnobug:fix/issue-1642-skip-unused-task-timing-tail

Conversation

@zmnobug

@zmnobug zmnobug commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a 16-byte device-phase header and publish whether any task-timing slot
    was dispatched from the last AICPU thread at the existing completion gate.
  • Read back the header and fixed phase prefix first, and skip the optional
    task-timing tail D2H copy when no task was tagged.
  • Use typed POD storage for the shared buffer in onboard and both simulators,
    with direct tests for the no-tail-read path and the wire layout.
  • Keep the a2a3/a5 and host-build-graph/tensormap runtime paths aligned and
    document the split readback layout.

Testing

  • Editable runtime build
  • C++ unit tests (86 targets; socket-backed test passed outside sandbox)
  • Full a2a3sim scene-test suite (137 selected)
  • Full a5sim scene-test suite (108 selected)
  • Pre-commit hooks for all changed files
  • Onboard hardware tests (arch precheck could not identify silicon because
    npu-smi returned empty Chip/NPU names)

Fixes #1656

@coderabbitai

coderabbitai Bot commented Aug 6, 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 Plus

Run ID: ac5227db-c472-4283-9737-dc1770b00200

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

The device phase buffer now contains a header, phase records, and an optional task-timing tail. AICPU cleanup publishes tail usage. Host runners read the tail only when the header indicates dispatched task timing data.

Changes

Task-timing tail readback

Layer / File(s) Summary
Shared phase-buffer layout
src/common/platform/include/common/device_phase.h, src/common/platform/include/aicpu/device_phase_aicpu.h
The buffer gains a header, typed storage, accessors, reset logic, and tail-usage detection for dispatched tasks, including incomplete tasks.
Device tail-usage publication
src/a2a3/runtime/..., src/a5/runtime/..., src/a2a3/platform/onboard/aicpu/kernel.cpp, src/a5/platform/onboard/aicpu/kernel.cpp
AICPU cleanup paths publish tail usage before completion and runtime teardown.
Host buffer integration
src/a2a3/platform/sim/host/device_runner.cpp, src/a5/platform/sim/host/device_runner.cpp, src/common/platform/onboard/host/device_runner_base.*
Host runners use shared buffer storage and conditionally copy and resolve the task-timing tail.
Validation and documentation
tests/ut/cpp/a2a3/test_task_timing_slots.cpp, docs/dfx/device-phases.md
Tests and documentation cover layout, reset behavior, conditional reads, and incomplete dispatches.

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

Sequence Diagram(s)

sequenceDiagram
  participant AICPUExecutor
  participant DevicePhaseBuffer
  participant HostDeviceRunner
  AICPUExecutor->>DevicePhaseBuffer: Publish task-timing tail usage
  HostDeviceRunner->>DevicePhaseBuffer: Read header and phase prefix
  HostDeviceRunner->>DevicePhaseBuffer: Read tail when usage is set
  HostDeviceRunner->>HostDeviceRunner: Resolve task-timing records
Loading

Possibly related PRs

Poem

I’m a rabbit with a header to read,
Skipping empty tails at top speed.
Dispatch marks the way,
Incomplete slots still stay,
Then clean spans hop home as agreed.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 39.47% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: skipping unused task-timing tail readback.
Description check ✅ Passed The description directly explains the header, conditional tail readback, aligned runtime changes, and tests.
Linked Issues check ✅ Passed The changes satisfy issue #1656 by marking tail usage, shifting the buffer layout, and gating tail readback across required runtime paths.
Out of Scope Changes check ✅ Passed The changes remain within issue #1656 and support its buffer layout, runtime integration, documentation, and test requirements.

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.

@zmnobug
zmnobug force-pushed the fix/issue-1642-skip-unused-task-timing-tail branch 2 times, most recently from d144004 to 61be4dc Compare August 6, 2026 07:09
- Add a fixed buffer header and publish tail usage from the last AICPU
  thread at the existing completion gates.
- Read back the header and phase prefix first; copy and resolve the
  optional tail only when a tagged task was dispatched.
- Use typed POD storage across onboard and both simulators to preserve
  the layout without violating C++17 object-lifetime rules.
- Cover layout, reset, and incomplete dispatch, and directly verify an
  unused header never invokes the tail read.

Refs hw-native-sys#1656
@ChaoZheng109
ChaoZheng109 force-pushed the fix/issue-1642-skip-unused-task-timing-tail branch from 61be4dc to 3f1847e Compare August 6, 2026 11:40
@ChaoZheng109
ChaoZheng109 merged commit 9435e0d into hw-native-sys:main Aug 6, 2026
19 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] Task-timing tail is copied D2H every profiling run even when no task is tagged

2 participants