Refactor: drop unused READY/RUNNING task states#770
Merged
Conversation
There was a problem hiding this comment.
Code Review
This pull request simplifies the task state machine by removing the explicit READY and RUNNING states from the PTO2TaskState enumeration. Tasks now remain in the PENDING state from submission through dispatch until they reach the COMPLETED state. Readiness is now determined by fanin_refcount, and the running status is tracked via per-core pointers. The changes include updates to documentation, the scheduler logic, and unit tests to align with this streamlined lifecycle. I have no feedback to provide.
The PTO2TaskState enum advertised the chain PENDING -> READY -> RUNNING -> COMPLETED -> CONSUMED, but RUNNING was never written or read in src/ and READY was written by a single CAS in release_fanin_and_check_ready with no readers. The fetch_add already elects a unique winner for the queue push, so the CAS was redundant. - Drop PTO2_TASK_READY and PTO2_TASK_RUNNING; renumber the enum to PENDING(0) / COMPLETED(1) / CONSUMED(2). The < / >= COMPLETED comparisons in scheduler/orchestrator still hold under the new numbering since CONSUMED remains the largest value. - Drop the redundant CAS PENDING->READY in the profiling-instrumented release_fanin_and_check_ready so it matches the non-profiling path. - Delete the unused task_state_name() helper (no callers in repo). - Update scheduler_cold_path stall-dump comment to reflect that task_state has no intermediate values. - Update unit tests to use PENDING in place of READY/RUNNING fixtures (semantically equivalent: anything < COMPLETED). - Sync docs/orchestrator.md and docs/scheduler.md state machines. Run on a2a3 ut-cpp: test_scheduler_state, test_task_state, test_wiring all pass.
1557539 to
3411163
Compare
jvjhfhg
approved these changes
May 13, 2026
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
The
PTO2TaskStateenum advertised the chainPENDING → READY → RUNNING → COMPLETED → CONSUMED, but in practice:PTO2_TASK_RUNNINGwas never written or read anywhere insrc/.PTO2_TASK_READYwas written by a single CAS in the profiling-instrumentedrelease_fanin_and_check_readyand had no readers. Thefanin_refcount.fetch_addalready elects a unique winner for the ready-queue push, so the CAS was redundant; the non-profiling overload had already been doing this without writingtask_state.The actual transitions performed by the runtime are
PENDING → COMPLETED → CONSUMED; ready-vs-running is derived fromfanin_refcount/ per-corerunning_slot_state, not fromtask_state.Changes
PTO2_TASK_READYandPTO2_TASK_RUNNING; renumber the enum toPENDING(0) / COMPLETED(1) / CONSUMED(2). The< / >= PTO2_TASK_COMPLETEDordering comparisons in scheduler/orchestrator still hold under the new
numbering since
CONSUMEDremains the largest value.PENDING → READYin the profiling-instrumentedrelease_fanin_and_check_readyso it matches the non-profiling path.task_state_name()helper (no callers in repo).scheduler_cold_pathstall-dump comment to reflect thattask_statehas no intermediate ready/running value.
PENDINGin place ofREADY/RUNNINGfixtures(semantically equivalent: anything
< COMPLETED).docs/orchestrator.md,docs/scheduler.md, andsrc/{a2a3,a5}/runtime/tensormap_and_ringbuffer/docs/RUNTIME_LOGIC.md.Applied to both
a2a3anda5runtime copies.Test plan
cmake --build build/ut_cpp --target test_scheduler_state test_task_state test_wiringctest -R 'scheduler_state|task_state|wiring'— 3/3 passrg 'PTO2_TASK_(READY|RUNNING)|task_state_name'— zero hits acrosssrc/,tests/,docs/