Fix: size HBG ready queues from reachable tasks by TaoZQY · Pull Request #1982 · hw-native-sys/simpler

TaoZQY · 2026-08-24T07:37:48Z

Summary

Size each of the nine HBG scheduler ready queues from the number of tasks
that can reach it during bind, instead of giving every queue the same
logical capacity of 8192.
Raise the arena reservation ceiling to 32768 slots per queue so the known
Qwen3 decode workload (9400 reachable AIC tasks) is covered.
Reject a graph during host bind with structured
SIMPLER_ERROR_READY_QUEUE_OVERFLOW diagnostics when any single queue's
reachable population exceeds that ceiling.
Apply the same implementation to a2a3 and a5, with regression tests and
updated error-code documentation.

Motivation

enqueue_ready has no retry or back-pressure path: if a queue is full, the
run fails. Queue capacity is therefore a correctness bound rather than only a
performance tuning value.

The Qwen3-14B decode GraphExecutionBatch16Seq3500 workload has 9400 tasks
that can reach ready_queue[AIC], exceeding the previous fixed logical
capacity of 8192. Its measured peak occupancy is currently much lower, so the
failure is latent, but a wider DAG or a slower consumer can expose it.

Implementation

Count the host task window once at bind time.
Count each distinct Graph Definition once, cache its per-queue population,
and add it for every submission that references the definition.
Count predicated tasks toward both the dummy queue and their shape queue,
because the predicate is evaluated on device.
Round each logical queue capacity up to a power of two (minimum 2) and pass
the nine derived capacities to the device through SchedulerLayout.
Keep the physical reservation bounded at 32768 slots per queue. Populations
above that limit fail before device execution and report the same error via
the return status, shared scheduler status, logs, and DFX path.
Keep population derivation host-only so it is not compiled into the AICPU
runtime, and remove the redundant/confusable reservation-capacity field.

Memory and performance trade-off

The physical reservation for the nine queues grows from
9 x 8192 x 24 = 1,769,472 bytes (~1.7 MB) to
9 x 32768 x 24 = 7,077,888 bytes (~7.1 MB) per reusable worker arena.
Logical capacities remain demand-derived, so small graphs seed only the slots
they can use even though the maximum reservation is larger.

Same-device a2a3 onboard A/B for paged_attention/small1, 50 rounds through
task-submit:

Metric (median)	`main` (`d90c574e`)	This PR (`2c92429a`)	Change
Host wall	783.8 us	554.6 us	-29.2%
Device wall	392.9 us	163.8 us	-58.3%
Bind control plane	28 us	27 us	no regression

The small-graph improvement comes from reducing its logical queue capacity
from 8192 to 2, which removes unnecessary device-side slot seeding.

Validation

C++ no-hardware unit tests: 122/122 passed.
a2a3 HBG simulation: 15 passed, 7 skipped.
a5 HBG simulation: 12 passed.
paged_attention/small1 a2a3 onboard golden test passed through
task-submit (task_20260826_195127_9682646898).
Qwen3-14B decode GraphExecutionBatch16Seq3500 onboard golden test passed
3/3 through task-submit (task_20260824_003633_278330810026).
Pre-commit hooks and all required GitHub CI checks passed.

Fixes#1920

coderabbitai · 2026-08-24T07:38:14Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d1600641-2c15-403f-80fc-b8878f62b32c

📥 Commits

Reviewing files that changed from the base of the PR and between 66ba5c4 and 33d6f14.

📒 Files selected for processing (9)

src/a2a3/runtime/host_build_graph/host/runtime_maker.cpp
src/a2a3/runtime/host_build_graph/runtime/runtime_types.h
src/a2a3/runtime/host_build_graph/runtime/scheduler/scheduler.h
src/a2a3/runtime/host_build_graph/runtime/shared/runtime_init.cpp
src/a5/runtime/host_build_graph/host/runtime_maker.cpp
src/a5/runtime/host_build_graph/runtime/runtime_types.h
src/a5/runtime/host_build_graph/runtime/scheduler/scheduler.h
src/a5/runtime/host_build_graph/runtime/shared/runtime_init.cpp
tests/ut/cpp/common/test_hbg_ready_queue_seed.cpp

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The host build graph now computes ready-queue populations from submitted tasks and graph definitions. It derives per-queue capacities, rejects populations above the configured limit, and uses the capacities when reserving and initializing scheduler queues.

Changes

Ready-queue capacity derivation

Layer / File(s)	Summary
Population model and capacity derivation `src/a2a3/runtime/host_build_graph/runtime/...`, `src/a5/runtime/host_build_graph/runtime/...`	The fixed queue-size macro is replaced by `READY_QUEUE_CAPACITY_LIMIT`. New population and capacity structures track task categories and resource shapes, enforce the limit, and derive power-of-two capacities.
Host population collection `src/a2a3/runtime/host_build_graph/host/runtime_maker.cpp`, `src/a5/runtime/host_build_graph/host/runtime_maker.cpp`	Host orchestration validates `total_tasks`, counts submitted task populations, aggregates graph-definition populations, and publishes derived capacities or the latched overflow status.
Scheduler reservation and validation `src/a2a3/runtime/host_build_graph/runtime/...`, `src/a5/runtime/host_build_graph/runtime/...`, `tests/ut/cpp/common/test_hbg_ready_queue_seed.cpp`	Scheduler reservations and queue headers use individual derived capacities. Tests cover Qwen sizing, multi-queue derivation, and the capacity-limit boundary.

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

Merge Risk:⚪ Minimal · up to 33d6f

The change sizes HBG ready queues from reachable tasks and rejects graphs above the supported bound; reported tests and performance checks pass, so no actionable merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
participant HostOrchestration
participant GraphDefinitions
participant ReadyQueuePopulations
participant SchedulerLayout
participant Scheduler
HostOrchestration->>ReadyQueuePopulations: Count submitted task-slot populations
HostOrchestration->>GraphDefinitions: Bind uploaded definitions
GraphDefinitions->>ReadyQueuePopulations: Add definition node populations
ReadyQueuePopulations->>SchedulerLayout: Derive per-queue capacities
SchedulerLayout->>Scheduler: Reserve and initialize queue storage

Poem

I’m a rabbit with queues in a row,
Counting each task as I go.
Capacities bloom,
Overflow finds room—
And schedulers ready to flow.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided, so the change rationale and validation details are not documented in the description.	Add a concise description covering bind-time ready-queue sizing, capacity overflow handling, mirrored a2a3/a5 changes, and validation results.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy issue `#1920` by deriving per-queue capacities, counting reachable tasks, enforcing the reservation ceiling, and failing during bind on overflow.
Out of Scope Changes check	✅ Passed	The changes remain within issue `#1920` scope and support the stated implementation with mirrored code and focused tests.
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Title check	✅ Passed	The title clearly and concisely describes the main change: sizing HBG ready queues from bind-time reachable task populations.

Fix all pre-merge checks with AI

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.}

ChaoZheng109

Review of the implementation, taking the counting method from #1920 as given -- everything below is independent of how the population is computed.

Verified and not commented on, since it is all correct: the routing in add_task matches push_ready_routed exactly, including counting a predicated task toward both dummy and its shape queue (the right conservative treatment, since the predicate resolves on device); the per-Definition histogram is walked once and cached by full_key; the saturating add uses LIMIT + 1 as a sentinel so uint64 cannot wrap; capacity_for_population(32768) lands exactly on the reservation; hoisting the total_tasks range check ahead of the new scan is necessary rather than incidental; and a2a3/a5 are line-for-line mirrors.

Seven comments below. The first three I would treat as blocking (error-code contract vs. docs, structured error reporting, and no coverage of the derive -> H2D -> init path); the other four are worth resolving before merge.

- Derive bounded per-queue capacities from bind-time reachability - Report oversized graphs consistently through host and shared status - Keep host-only sizing out of AICPU and mirror a2a3/a5 behavior - Cover initialization, saturation, rejection, and support docs

TaskKind::GRAPH_NODE encoded where a task belongs rather than what it is. A task in a Graph body is scheduled exactly like one submitted outside a Graph — same active_mask, logical_block_num and ResourceShape queue — so its kind is now the ordinary KERNEL or DUMMY, and the Graph it belongs to is named by graph_context alone. That is the vocabulary task_id_encoding.h already uses for the same distinction, where a task id is either GLOBAL or IN_GRAPH. - TaskKind is {KERNEL, DUMMY, GRAPH}: two leaves plus the container - complete_task routes on graph_context, which is null for every task outside a Graph and so short-circuits before the kind is read - graph_execution_from_slot is gone: its only caller had already established both conditions it re-checked, which made its null return unreachable ActiveMask::is_dummy() becomes the sole definition of dummy, so ResourceShape::DUMMY and TaskKind::DUMMY can no longer disagree. The kind was derived from the whole raw_ byte while the shape used core_mask() alone, leaving the two equal by coincidence rather than by construction. The ready-queue sizing hw-native-sys#1982 added passes a kind for every member of a Definition and passed `TaskKind::GRAPH_NODE`, so it moves with the enumerator. It now derives the kind the way materialize does -- dummy by `ActiveMask::is_dummy()`, else KERNEL. That preserves behaviour: `ReadyQueuePopulations::add_task` singles out `TaskKind::GRAPH` and routes everything else by `active_mask.to_shape()`, so `GRAPH_NODE` there only ever meant "not the shell". Its unit test carried the same enumerator and takes KERNEL, which for the same reason leaves every capacity assertion unchanged.

host_build_graph called a task inside a Graph body a "node", which encodes membership as a type. task_id_encoding.h had already moved to the right model -- TaskIdSpace{GLOBAL, IN_GRAPH} and make_in_graph_task(graph_local_id, task_index) -- so the two vocabularies collided line by line, most visibly in graph_execution.h where GRAPH_MAX_NODES bounded IN_GRAPH_TASK_INDEX_BITS. Everything this runtime schedules is a task; the id's high bits say whether it belongs to a Graph or stands on its own. So the body's types, constants, fields, cursors, free functions, comments, docs and tests all name a task: GRAPH_MAX_NODES -> MAX_IN_GRAPH_TASKS GRAPH_MATERIALIZE_SLICE_NODES -> GRAPH_MATERIALIZE_SLICE_TASKS GraphNodeDefinition -> InGraphTaskDefinition GraphNodeStorage -> InGraphTaskStorage GraphRecordedNode -> RecordedInGraphTask slot.graph_node_index -> slot.in_graph_task_index off_nodes / off_node_offsets -> off_in_graph_tasks / ..._offsets remaining_nodes, retired_nodes, published_nodes, materialized_nodes, constructed_nodes -> ..._tasks record_node, graph_execution_complete_node, retire_node -> ..._in_graph_task HostPhaseKind::OrchRecordNode -> OrchRecordInGraphTask Short members inside GraphExecution / GraphRecording drop the qualifier (node_count -> task_count, node_at -> task_at, node_tensor_pool -> task_tensor_pool): the owning type already fixes the layer, and GraphDefinition::task_count already meant the inner count. Prose cannot take that shortcut. "task" alone names three things here -- a global task holding a task-table slot, the Graph task that is one of them, and an in-graph task living in the Graph's own storage -- so a blanket rename would trade one unambiguous word for a three-way ambiguous one. Each comment now says which layer it means; a bare "task" appears only where the enclosing type or function has already fixed it, and a sentence spanning two layers qualifies both. The recorder is a fourth context: while it builds one, the thing is a *recorded* task, since no Graph owns it yet and no Definition exists. Two comments named the wrong layer rather than a stale one. CHIP_MAX_FANIN was said to bound "a ring task's inline fanin", but hbg has no task ring -- its task table is whole-graph-resident and it reads none of the RUNTIME_ENV_RING_COUNT slots -- so that is a global task's inline fanin, the contrast TaskDescriptor::fanin already draws. GraphPrepare's tasks_processed and HostPhaseRecord's payload counted "nodes"; both count in-graph tasks. Four comments in the recorder described a mechanism that is gone: reset() claimed to preserve a `tensors` member the recording has not had since its tensors moved into a per-thread pool, and the record path twice explained address validity by a move into recording.tasks. Those addresses point into that pool, which is allocated at the cap and never grows. Two more narrated a deleted push_back and are dropped. The phase name is the one rename with a compatibility cost. Its enumerator keeps its ordinal, so the uint32 HostPhaseRecord::kind on the wire is unchanged, but an unrecognised phase *string* is attributed to host_main rather than rejected -- so accepting only the new spelling would silently redraw every log written before this commit, including the archived runs cited in docs/investigations/, with the recorder's work on the main lane. strace_timing.py therefore accepts both names behind one named set and a test feeds the old one; the three investigation entries say up front which names the tooling emitted at the time, and the index line carries both so either spelling finds the entry. The consumers that had to move with the producer -- strace_timing.py, the swimlane converter's Graph-instance decoding and its trace args, both unit tests, the profiling docs -- are all here. Two things belong to the graph_context change one commit earlier and land here only because stating them needs this commit's vocabulary. complete_task's `task_kind == GRAPH` clause had no test: it is what keeps the outer Graph task out of the in-graph path, and that slot is the one place where a non-null graph_context is a GraphDefinition rather than a GraphExecution, so losing the clause means reading one struct's bytes as another's with no fault and no error code. The new test fails if the clause is removed and nothing else in the suite does. The invariant behind it is now stated where graph_context is declared, since it spans two fields and no single accessor checks it any more. The ready-queue sizing hw-native-sys#1982 added reads the Definition's task array, so its `GraphNodeDefinition` / `off_nodes` / `nodes` follow the same renames as every other reader. Its error message named a "node array". No layout or ABI change: the off_* fields keep their device-image offsets, in_graph_task_index stays in ChipTaskSlotState's tail padding, and the 40-byte descriptor is untouched.

…rship in graph_context (#2051) host_build_graph described the contents of a Graph body as "nodes" while task_id_encoding.h already called the same thing an in-graph task, and it encoded *where* a task belongs into TaskKind, which otherwise says *what* a task is. This aligns both. Membership is not a kind TaskKind::GRAPH_NODE did two things and nothing else: route a completion to the Graph counters, and fetch the GraphExecution off the slot. A task inside a Graph body is scheduled exactly like one submitted outside it -- same active_mask, logical_block_num and ResourceShape queue -- so its kind is now the ordinary KERNEL or DUMMY, and graph_context alone names the Graph it belongs to. TaskKind is {KERNEL, DUMMY, GRAPH}: two leaves plus the container. complete_task routes on graph_context, which is null for every task outside a Graph and therefore short-circuits before the kind is read. Three states stay distinguishable -- no graph_context is an ordinary task, graph_context with GRAPH is the shell, graph_context without it is a task in the body. graph_execution_from_slot is gone: its only caller had already established both conditions it re-checked, so its null return was unreachable. That leaves the `task_kind == GRAPH` half of the predicate carrying the whole distinction, on the one slot where a non-null graph_context is a GraphDefinition and not a GraphExecution -- before localize swaps it. Lose the clause and the static_cast reads one struct's bytes as another's, with no fault and no error code, so it now has a test that fails when the clause is removed and that nothing else in the suite duplicates. The invariant -- which of the two structs graph_context points at, and why every reader must test task_kind first -- is stated where the field is declared, since it spans two fields and no accessor checks it any more. The ready-queue sizing #1982 added passes a kind for every member of a Definition and passed TaskKind::GRAPH_NODE, so it now derives the kind the way materialize does: dummy by ActiveMask::is_dummy(), else KERNEL. That preserves behaviour -- ReadyQueuePopulations::add_task singles out TaskKind::GRAPH and routes everything else by active_mask.to_shape(), so GRAPH_NODE there only ever meant "not the shell", which leaves every capacity assertion unchanged. One definition of dummy TaskKind::DUMMY was derived from the whole ActiveMask::raw_ byte while ResourceShape::DUMMY used core_mask() alone -- equal only by coincidence, and silently divergent the moment anyone adds a bit above the low three. ActiveMask::is_dummy() is now the sole definition and both derive from it. A dep-only task inside a Graph body is also marked DUMMY now, which GRAPH_NODE used to mask. Vocabulary Types and constants take the in-graph-task name, which separates them from the Graph as a whole (GraphDefinition, GraphExecution) and from the shell task: GRAPH_MAX_NODES -> MAX_IN_GRAPH_TASKS GRAPH_MATERIALIZE_SLICE_NODES -> GRAPH_MATERIALIZE_SLICE_TASKS GraphNodeDefinition -> InGraphTaskDefinition GraphNodeStorage -> InGraphTaskStorage GraphRecordedNode -> RecordedInGraphTask slot.graph_node_index -> slot.in_graph_task_index off_nodes / off_node_offsets -> off_in_graph_tasks / ..._offsets record_node / retire_node / graph_execution_complete_node -> ..._in_graph_task Short members inside GraphExecution / GraphRecording drop the qualifier (node_count -> task_count, node_at -> task_at, the argument pools, and the remaining/retired/published/materialized/constructed counters): the owning type already names the Graph, and GraphDefinition::task_count already meant the inner count -- previously that struct carried both task_count and off_nodes for one concept. Prose cannot take that shortcut. "task" alone names three things here -- a global task holding a task-table slot, the Graph task that is one of them, and an in-graph task living in the Graph's own storage -- so a blanket rename would trade one unambiguous word for a three-way ambiguous one. Each comment now says which layer it means; a bare "task" appears only where the enclosing type or function has already fixed it. The recorder is a fourth context: while it builds one, the thing is a *recorded* task, since no Graph owns it yet and no Definition exists. Two comments named the wrong layer rather than a stale one. CHIP_MAX_FANIN was said to bound "a ring task's inline fanin", but hbg has no task ring -- its task table is whole-graph-resident and it reads none of the RUNTIME_ENV_RING_COUNT slots -- so that is a global task's inline fanin. GraphPrepare's tasks_processed and HostPhaseRecord's payload counted "nodes"; both count in-graph tasks. Four more comments in the recorder described a mechanism that is gone -- a `tensors` member reset() claimed to preserve, and address validity explained by a move into recording.tasks -- and are corrected or dropped. The profiling phase name, and why both spellings stay readable HostPhaseKind::OrchRecordNode becomes OrchRecordInGraphTask and the phase name it emits becomes "record_in_graph_task". Its enumerator keeps its ordinal, so the uint32 HostPhaseRecord::kind on the wire is unchanged and existing traces still decode. The string is a different matter: an unrecognised phase name is *attributed to host_main*, not rejected, so dropping the old spelling would silently redraw every pre-change log's recorder work onto the wrong lane -- including the archived runs cited in docs/investigations/. strace_timing.py therefore accepts both names behind one named set and a test feeds the old one. The three investigation entries say up front which names the tooling emitted at the time, and the index line carries both so either spelling finds the entry. Serialized names that change The swimlane converter's Graph-instance trace args: visible_node_count -> visible_in_graph_task_count, visible_node_index_min|max likewise, the synthetic_id_layout value, and the event name. Nothing in the repo but its unit test consumes them, but they are output names (codestyle rule 10 Tier C). Rider: lock_guard -> scoped_lock graph_recorder_pool.h and host_phase_trace.cpp (both arches, 14 sites) switch to std::scoped_lock in the CTAD spelling orchestrator.cpp already used. This is not part of the rename: pre-commit's clang-tidy checks whole translation units, so neither file could carry its comment changes while its existing std::lock_guard uses tripped modernize-use-scoped-lock. Behaviour is identical for a single mutex. No layout or ABI change: the off_* fields keep their device-image offsets, in_graph_task_index stays in ChipTaskSlotState's tail padding, and the 40-byte descriptor is untouched. Verification - Six runtime variants build; 122/122 cpput; 2012 passed / 11 skipped pyut - a2a3sim: host_build_graph_validation 5/5, graph_predicated_dispatch pass - a2a3 onboard (every run under a task-submit device lock): hbg full scene tests + wide_dispatch 44 passed / 1 skipped, tensormap_and_ringbuffer full scene tests 46 passed / 1 skipped, deepseek_v4_flash_decode 1 passed, no 507018 / deadlock / HandleTaskTimeout / FATAL in any device log - a5 onboard is not covered -- this box is a2a3 silicon, so onboard-arch-precheck refuses it; a5 has sim and cpput coverage only

TaoZQY force-pushed the codex/fix-issue-1920-ready-queue-capacity branch 2 times, most recently from 1a57472 to 56d4791Compare August 26, 2026 06:54

ChaoZheng109 reviewed Aug 26, 2026
View reviewed changes

TaoZQY force-pushed the codex/fix-issue-1920-ready-queue-capacity branch from 56d4791 to 2c92429Compare August 27, 2026 02:43

ChaoZheng109 approved these changes Aug 27, 2026
View reviewed changes

ChaoZheng109 merged commit ad0188b into hw-native-sys:mainAug 27, 2026
20 checks passed

poursoul mentioned this pull request Aug 27, 2026
Name the hbg Graph body after tasks, and hold its membership in graph_context #2051
Merged

Navigation Menu

Uh oh!

Fix: size HBG ready queues from reachable tasks - #1982

Fix: size HBG ready queues from reachable tasks#1982
ChaoZheng109 merged 1 commit into
hw-native-sys:mainfrom
TaoZQY:codex/fix-issue-1920-ready-queue-capacity

TaoZQY commented Aug 24, 2026•
edited
Loading

Uh oh!

coderabbitaiBot commented Aug 24, 2026•
edited
Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 inconclusive)

Uh oh!

ChaoZheng109 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

TaoZQY commented Aug 24, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Implementation

Memory and performance trade-off

Validation

Uh oh!

coderabbitaiBot commented Aug 24, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 inconclusive)

Uh oh!

ChaoZheng109 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TaoZQY commented Aug 24, 2026•
edited
Loading

coderabbitaiBot commented Aug 24, 2026•
edited
Loading