Skip to content

[Code Health] hbg: make PUBLISHED precede COMPLETED by construction and turn the progress byte into a sequential enum #2106

Description

@ChaoZheng109

Category

Technical Debt (cleanup, refactor)

Component

AICPU Scheduler

Description

Follow-up to #2095 (do after it merges, as one unified change, with a performance measurement).

The per-task progress byte (progress_flags[], bit0 = COMPLETED, bit1 = PUBLISHED) conceptually encodes a strictly linear progression — pending → published → completed; a task is in exactly one state at a time. The current implementation expresses that progression through bit-containment values (0x0 → 0x2 → 0x3) so that a lock-free fetch_or doubles as a monotone-max: the PUBLISHED bookkeeping write races the COMPLETED store, because the bookkeeping runs after the final MMIO token write while the FIN → completion chain forks off that same token write. A plain store with sequential enum values (0/1/2) would let a late publish write regress an already-completed byte, which livelocks the wake-list sentinel protocol.

The cleaner long-term shape is ordering by construction instead of monotonicity by encoding: hoist the final publisher's bookkeeping (the total-reaching fetch_add + PUBLISHED store, in record_published_blocks) to before its batch's MMIO token writes (after payload prepare, before the token flush). That closes the causal chain — PUBLISHED ≺ token ≺ FIN ≺ COMPLETED — so the byte can become a genuine sequential enum written with plain stores, matching the mental model task_state already follows (whose PENDING → COMPLETED → CONSUMED transitions are all causally serialized; TMR additionally guards COMPLETED → CONSUMED with an expected-value CAS).

Safety argument already worked out in review: cores are claimed at prepare time, before any bookkeeping, so setting PUBLISHED ahead of the token writes cannot let a staged consumer occupy cores the producer's in-flight blocks need; and with concurrent publishers, the total-reaching thread's bit store still precedes its own tokens, whose FINs gate the all-FIN completion, so the chain holds regardless of sibling threads' token timing.

Scope of the unified change:

  • Split the publish accounting around the token flush at the three publish sites (dispatch_shape, stage_consumer_blocks, stage_sync_start_cores), both arches: pre-account before the flush, seal after.
  • Re-encode the progress byte as a sequential enum (e.g. TASK_PROGRESS_NONE/PUBLISHED/COMPLETED), replacing TASK_FLAG_* and the fetch_or with plain stores; update the byte-level readers (is_completion_flag_set / is_publish_flag_set) to ordered comparisons.
  • Re-audit the ED publish-list seal placement (it may stay after the flush; only the bit/count ordering must move).
  • Measure: dispatch-path cost before/after (the change resequences the hot publish loop), via the qwen device A/B protocol on pinned dies plus the QPROBE/swimlane probes, and hbg-bind-phases to confirm host side untouched.

Until then the fetch_or encoding is correct and validated; this issue is about expressing the state model directly rather than through an encoding trick.

Location

  • src/common/host_build_graph/shared_memory.h (progress_flags, TASK_FLAG_*, byte helpers)
  • src/{a2a3,a5}/runtime/host_build_graph/runtime/scheduler/scheduler.h (record_published_blocks, seal_ed_publish_list)
  • src/{a2a3,a5}/runtime/host_build_graph/runtime/scheduler/scheduler_dispatch.cpp / scheduler_completion.cpp (publish sites around flush_publish)

Priority

Low (no impact today, good to fix eventually)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    code healthTechnical debt, robustness, code quality

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions