Fix: fold simulated device logs into HostLogger - #2061
Conversation
indigo1973
commented
Aug 28, 2026
indigo1973
commented
- Bind each simulation AICPU DSO to the process-owned HostLogger state before applying the compatibility log level.
- Route simulation device records through HostLogger so they share the host envelope, live threshold, and configured destination.
- Remove the duplicate simulation formatter/writer and its 2048-byte PIPE_BUF constraint while leaving the onboard CANN backend unchanged.
- Add live-threshold, envelope, concurrent-thread, and forked-process coverage for issue [Code Health] Log subsystem — one clock, one grammar, one gate #1792 item 5.
- Fully initialize existing ABI v2 log-directory fields for strict simulation AICPU builds.
- Bind each simulation AICPU DSO to the process-owned HostLogger state before applying the compatibility log level. - Route simulation device records through HostLogger so they share the host envelope, live threshold, and configured destination. - Remove the duplicate simulation formatter/writer and its 2048-byte PIPE_BUF constraint while leaving the onboard CANN backend unchanged. - Add live-threshold, envelope, concurrent-thread, and forked-process coverage for issue hw-native-sys#1792 item 5. - Fully initialize existing ABI v2 log-directory fields for strict simulation AICPU builds.
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughChangesThe AICPU simulation logger now delegates severity checks, state binding, level updates, formatting, and output to HostLogger-backed AICPU logging
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to Simulation and host records now target the same log destination, but separate buffered writers may produce out-of-order or delayed diagnostic records during concurrent, reload, or failure scenarios. The change is otherwise mergeable with explicit owner awareness and follow-up on shared-sink serialization and flush behavior. Sequence Diagram(s)sequenceDiagram
participant device_runner
participant AICPU_sim_device_log
participant HostLogger
device_runner->>AICPU_sim_device_log: set_host_log_state(state)
AICPU_sim_device_log->>HostLogger: bind_state(state)
device_runner->>AICPU_sim_device_log: set_log_level(level)
AICPU_sim_device_log->>HostLogger: set_level(level)
AICPU_sim_device_log->>HostLogger: vlog(level, message)
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 22.58% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 31 functions across 8 files. (3 skipped: 3 unsupported.) 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 |
Every DSO that compiles host_log.cpp owns a private buffered stream on the process's host.<pid>.log, and HostLogFileSink held that FILE* with no destructor. A dlopened module's pending records were therefore discarded with its mapping: dlclose unmaps the buffer without flushing it. Folding the simulated device log into HostLogger makes the sim AICPU SO such a module, and a high-volume one. DeviceRunner::unload_executor_binaries() dlcloses it on every teardown, so its DEBUG/INFO/TIMING records — which ride the buffer, since only WARN and above write through — were lost whenever a bound output_prefix sent them to the file. The generated orchestration SO has the same shape and was already exposed. Give the sink a destructor. dlclose runs it, so the tail reaches disk. It closes only a stream this process opened: an inherited stream holds records the parent still owns, and flushing that copied buffer would duplicate them. test_host_log_dso_unload pins the contract with a consumer .so that emits 200 INFO records and is unloaded; 159 of them reach the file without the destructor. test_sim_device_log gains the missing case for the bound log directory, which had no coverage on the sim path at all. Docs: logging.md described sim as an independent stderr backend with its own level flags and no host prefix, and pointed at hw-native-sys#1792 item 5 as outstanding work. host-trace.md described the file buffering as per process; it is per module, which is what makes unload observable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
