Update: improve scope stats reporting#992
Conversation
📝 WalkthroughWalkthroughThis PR extends the scope-stats collection system to track dependency-pool occupancy alongside existing task-window, heap, and tensormap metrics. Changes span data contracts (adding dep_pool fields to ScopeStatsRecord and ScopeStatsDataHeader), scheduler snapshot publication infrastructure, runtime capture during scope boundaries, collector ABI/storage updates, JSONL serialization (schema version 5), and an extensive HTML report generator refactor with interactive charts and risk-metric visualization. ChangesScope-stats dependency-pool resource integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Code Review
This pull request extends the scope_stats diagnostic feature to track and visualize scheduler dependency-list pool (dep_pool) usage, updating the schema to version 5. Key changes include capturing dep-pool snapshots in the runtime and scheduler, passing these metrics through the AICPU collector, and enhancing the HTML report generator with new chart stacks, a formula guide, and a top peaks table. The review feedback highlights several improvement opportunities, including resolving a potential ValueError crash in the plotting tool when finding the minimum of an empty sequence, defensively using .get() to prevent KeyError on malformed records, adding explicit padding in the ScopeStatsDataHeader shared memory structure to avoid compiler-dependent misalignment, and replacing angle brackets in Markdown placeholders to prevent shell syntax errors.
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.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
simpler_setup/tools/scope_stats_plot.py (1)
124-138:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse LIFO pairing for same-site nested scopes.
At Line 135, FIFO matching (
pop(0)) mispairs begin/end when the samesiteis nested (e.g., recursive/re-entrant scope usage), which corrupts derived metrics and peak attribution. Use stack semantics (pop()).Suggested fix
- queue = pending[ring].get(site) - if queue: - pairs[ring].append((queue.pop(0), rec)) + stack = pending[ring].get(site) + if stack: + pairs[ring].append((stack.pop(), rec))🤖 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 `@simpler_setup/tools/scope_stats_plot.py` around lines 124 - 138, The pairing logic in _pair_by_ring uses FIFO queue semantics (pending[ring][site].pop(0)) which mispairs nested/re-entrant same-site scopes; change it to LIFO stack semantics by using pending[ring][site].pop() when matching an "end" record (update the code around the queue variable and the pop call) so the most recent "begin" pairs with the current "end".
🤖 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.
Outside diff comments:
In `@simpler_setup/tools/scope_stats_plot.py`:
- Around line 124-138: The pairing logic in _pair_by_ring uses FIFO queue
semantics (pending[ring][site].pop(0)) which mispairs nested/re-entrant
same-site scopes; change it to LIFO stack semantics by using
pending[ring][site].pop() when matching an "end" record (update the code around
the queue variable and the pop call) so the most recent "begin" pairs with the
current "end".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7a6cfc96-6ea0-482e-9861-f5e028037419
📒 Files selected for processing (18)
docs/dfx/scope-stats.mdsimpler_setup/scene_test.pysimpler_setup/tools/scope_stats_plot.pysrc/a2a3/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_orchestrator.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/pto_scheduler.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/pto_runtime2_init.cppsrc/a5/runtime/tensormap_and_ringbuffer/aicpu/aicpu_executor.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_orchestrator.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/pto_scheduler.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/shared/pto_runtime2_init.cppsrc/common/platform/include/aicpu/scope_stats_collector_aicpu.hsrc/common/platform/include/common/scope_stats.hsrc/common/platform/include/host/scope_stats_collector.hsrc/common/platform/shared/aicpu/scope_stats_collector_aicpu.cppsrc/common/platform/shared/host/scope_stats_collector.cppsrc/common/task_interface/call_config.htests/st/a2a3/tensormap_and_ringbuffer/dfx/scope_stats/test_scope_stats.py
ac3e67a to
9baeb05
Compare
|
Addressed the CodeRabbit outside-diff item in 9baeb05: _pair_by_ring now uses LIFO matching for same-site nested scopes, with a unit test covering the nested pairing case. |
a9a12d7 to
828e5fc
Compare
Add dep_pool capacity and usage collection through host and AICPU scope_stats paths. Refresh the scope_stats HTML around top peaks, resource charts, hover details, and expanded chart views. Document report fields and TensorMap peak context handling in the existing English guide.
828e5fc to
9f94a78
Compare
Summary
Fixes #991