[Code Health] Warn when task fanin/fanout exceeds 16 during dependency construction - #4
Closed
yanghaoran29 wants to merge 1 commit into
Closed
[Code Health] Warn when task fanin/fanout exceeds 16 during dependency construction#4yanghaoran29 wants to merge 1 commit into
yanghaoran29 wants to merge 1 commit into
Conversation
…ruction Very large fanin/fanout is a code-health smell (see hw-native-sys#959): it makes the dependency graph hard to reason about and can hide an unexpectedly dense wiring shape. Nothing flagged it before. Closes hw-native-sys#1261. Add a two-part, bounded diagnostic, split by the phase where each degree is naturally constructed: - Fanout (pto_orchestrator.cpp, append_fanin_or_fail): the orchestrator increments each producer's fanout_count per claimed consumer. Emit an immediate WARN only on a new high-water above the threshold (computed under fanout_lock, logged after unlock — no logging in the critical section), and count each producer once at the 16->17 crossing. - Fanin (pto_scheduler.h, wire_task): scheduler thread 0 wires the task's fanin edges; warn on a new per-ring high-water and count each task. At program end, emit one summary line per metric: - "=== [Orchestrator] tasks with fanout>16: N ===" in mark_done() - "=== [Scheduler] tasks with fanin>16: N ===" in shutdown() (thread 0) The high-water gate keeps the AICPU device log from flooding on the hot path (codestyle rule 7); the over-threshold counters are accumulated silently and reset per run. Threshold PTO2_DEP_DEGREE_WARN_THRESHOLD=16. Purely diagnostic — no behavior change, no new env/macro gate. Mirrored to a2a3 and a5. Add a dummy_task st case (DenseFanoutFanin / case=4): one producer fanned out to 18 dummy barriers, then one consumer depending on all 18, so both degrees exceed the threshold. Verified on a2a3sim + a5sim that the WARNs fire only on new maxima (17/18/19) and the summaries report 1/1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes hw-native-sys#1261. Very large fanin/fanout is a code-health smell (cf. hw-native-sys#959) — it makes the dependency graph hard to reason about and can hide an unexpectedly dense wiring shape. Nothing flagged it before.
This adds a two-part, bounded diagnostic, split by the phase where each degree is naturally constructed:
1. Immediate WARN during execution (high-water gated)
Per codestyle rule 7 (never flood the AICPU device log on the hot path), each metric warns only on a new high-water max above the threshold:
pto_orchestrator.cpp::append_fanin_or_fail, where the orchestrator increments each producer'sfanout_countper claimed consumer. The value is captured underfanout_lockand logged after unlock (no logging in the critical section).pto_scheduler.h::wire_task, where scheduler thread 0 wires a task's fanin edges (wfanin), gated on a per-ring high-water.2. End-of-run summary
Every task/producer over the threshold is counted silently during the run, then one line per metric is emitted at teardown:
=== [Orchestrator] tasks with fanout>16: N ===inmark_done()=== [Scheduler] tasks with fanin>16: N ===inSchedulerContext::shutdown()(thread 0)Counters reset per run. Threshold
PTO2_DEP_DEGREE_WARN_THRESHOLD = 16. Purely diagnostic — no behavior change, no new env/macro gate. Mirrored to a2a3 and a5.Test
Adds a
dummy_taskst case (DenseFanoutFanin,case=4): one producer fanned out to 18 dummy barriers, then one consumer depending on all 18, so both degrees exceed the threshold.Verified on
a2a3sim+a5sim(both pass):18 consumers → only 3 WARN lines (one per new max) confirms the flood guard. Summaries need
--log-level v0; the WARNs show at the default level.Notes
PTO2_FANIN_INLINE_CAPis 64, so 16 is an advisory policy threshold, not a structural boundary.dcmiinit fails, arch precheck refuses); the sim runs exercise the identical orchestrator/scheduler code paths.🤖 Generated with Claude Code