Strip all LOG_* call sites — logging re-tiering baseline - #1447
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR reduces routine informational logging across a2a3 and a5 execution, scheduling, profiling, and diagnostic paths. It also adds collector scope cleanup, preserves unusable-device state after failed reset recovery, adjusts PMU teardown, and resolves task binary addresses using the CoreCallable binary-data offset. ChangesRuntime logging and cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
c0a7c39 to
e407c3e
Compare
e407c3e to
2c1b192
Compare
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 `@src/a2a3/platform/onboard/host/comm_hccl.cpp`:
- Line 203: Raise the periodic root-info wait message from DEBUG to LOG_INFO_V0
in both wait_for_rootinfo implementations:
src/a2a3/platform/onboard/host/comm_hccl.cpp lines 203-203 and
src/a5/platform/onboard/host/comm_hccl.cpp lines 208-208. Preserve the existing
message and timing behavior while ensuring it remains visible with DEBUG
disabled.
🪄 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 Plus
Run ID: 6204ff88-2d61-4963-b363-d8f7b97adb75
📒 Files selected for processing (41)
src/a2a3/platform/onboard/aicpu/kernel.cppsrc/a2a3/platform/onboard/host/comm_hccl.cppsrc/a2a3/platform/onboard/host/device_runner.cppsrc/a2a3/platform/onboard/host/host_regs.cppsrc/a2a3/platform/shared/aicpu/pmu_collector_aicpu.cppsrc/a2a3/platform/shared/host/pmu_collector.cppsrc/a2a3/platform/sim/host/device_runner.cppsrc/a2a3/runtime/host_build_graph/aicpu/aicpu_executor.cppsrc/a2a3/runtime/host_build_graph/host/runtime_maker.cppsrc/a2a3/runtime/host_build_graph/runtime/orchestrator_core/pto_orchestrator.cppsrc/a2a3/runtime/host_build_graph/runtime/scheduler/pto_scheduler.cppsrc/a2a3/runtime/host_build_graph/runtime/scheduler/scheduler_cold_path.cppsrc/a2a3/runtime/host_build_graph/runtime/scheduler/scheduler_dispatch.cppsrc/a2a3/runtime/host_build_graph/runtime/shared/pto_shared_memory.cppsrc/a2a3/runtime/host_build_graph/runtime/shared/pto_tensormap.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/host/runtime_maker.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_orchestrator.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/pto_scheduler.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_completion.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_dispatch.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/pto_shared_memory.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/pto_tensormap.cppsrc/a5/platform/onboard/aicpu/kernel.cppsrc/a5/platform/onboard/host/comm_hccl.cppsrc/a5/platform/onboard/host/device_runner.cppsrc/a5/platform/onboard/host/host_regs.cppsrc/a5/platform/shared/aicpu/pmu_collector_aicpu.cppsrc/a5/platform/shared/host/pmu_collector.cppsrc/a5/platform/sim/host/device_runner.cppsrc/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cppsrc/a5/runtime/tensormap_and_ringbuffer/host/runtime_maker.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_orchestrator.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/pto_scheduler.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_completion.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_dispatch.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/shared/pto_shared_memory.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/shared/pto_tensormap.cppsrc/common/platform/onboard/host/device_runner_base.cpp
💤 Files with no reviewable changes (10)
- src/a2a3/runtime/host_build_graph/aicpu/aicpu_executor.cpp
- src/a5/platform/onboard/aicpu/kernel.cpp
- src/a5/platform/onboard/host/device_runner.cpp
- src/common/platform/onboard/host/device_runner_base.cpp
- src/a2a3/platform/onboard/host/device_runner.cpp
- src/a2a3/platform/shared/host/pmu_collector.cpp
- src/a2a3/platform/onboard/aicpu/kernel.cpp
- src/a5/platform/sim/host/device_runner.cpp
- src/a5/platform/shared/host/pmu_collector.cpp
- src/a2a3/platform/sim/host/device_runner.cpp
2c1b192 to
52b6f80
Compare
Trim low-signal LOG_* call sites across the runtime and platform layers, keeping the diagnostics that trace program flow or failures: - Flow / lifecycle / summary logs kept at INFO; per-item (per-tensor, per-arg) detail demoted to LOG_DEBUG; failure paths stay LOG_ERROR / LOG_WARN. Loader / device-runner load-path diagnostics kept at their original levels. - AICPU hot paths (per-task / per-scope dispatch loops) carry no logs (codestyle rule 7); DFX collectors keep their buffer-level init/flush diagnostics and telemetry counters. - STRACE, LOG_RUNTIME_FAILURE and ACL logging left untouched. - Log-only support removed with its logs (bookkeeping counters, dump scaffolding); genuine out-of-scope strip collateral restored (unused-parameter removals, blank-line deletions). Rebased onto upstream/main; dense dependency diagnostics follow upstream's debug-only / removed decisions (hw-native-sys#1448), completion follows the polling mechanism (hw-native-sys#1435). Co-Authored-By: vegetabledoww <114059112+vegetabledoww@users.noreply.github.com>
Performance impact — no measurable changeSame-device serial benchmark on NPU device 8 (eliminates the device-to-device / neighbor-contention variance of a cross-device run), a2a3 / Base ( Average change per metric
Per-example Effective (µs)
Conclusion: all metrics move < 1% on average — within measurement noise. 6/8 examples are in noise; the two outliers are the smallest Case2 cases (~560 µs) moving in opposite directions (+5.3% / −2.0%), i.e. run-to-run jitter, not a systematic change. Expected for a logging-only change: default-off logs, |
Reduce logging footprint (#1428)
Curate
LOG_*call sites across the runtime and platform layers: droplow-signal logs, demote per-item detail to
LOG_DEBUG, and keep thediagnostics that trace program flow or failures.
Net effect vs base (individual call sites; a2a3 / a5 counted separately)
LOG_INFO_V0+ 8LOG_DEBUG+ 5LOG_INFO_V9+ 2LOG_ERROR+ 3LOG_WARN)LOG_DEBUGLOG_INFO_V0→LOG_DEBUG)Curation rules
per-arg) detail demoted to
LOG_DEBUG; failure paths stayLOG_ERROR/LOG_WARN. Loader / device-runner load-path diagnosticskept at their original levels.
(codestyle rule 7); DFX collectors keep their buffer-level init/flush
diagnostics and telemetry counters.
STRACE,LOG_RUNTIME_FAILUREand ACL logging left untouched.scaffolding); out-of-scope strip collateral restored (unused-parameter
removals, blank-line deletions).
Rebased onto
upstream/main; dense dependency diagnostics follow upstream'sdebug-only / removed decisions (#1448), completion follows the polling
mechanism (#1435).