feat(runtime): scheduler-driven full_refresh bit for page-table mirror#651
feat(runtime): scheduler-driven full_refresh bit for page-table mirror#651raikonenfnu wants to merge 4 commits into
Conversation
A fully concrete example (real sentence, real tokens)Let's make it painfully literal. Use a tiny System prompt (shared by everyone):
Tokenized (pretend each word/punct is one token): Request A = system prompt + With TimelineStep 1 — Request A runs first (cold). It computes its own KV into private pages: At this point page 0's data ("You are a helpful") is written into physical box Step 2 — scheduler publishes the shared prefix. Because "You are a helpful" is a clean, full page that other requests will reuse, the radix tree adopts a canonical copy of it: Step 3 — Request B arrives. Same system prompt, different question:
B's logical page 0 is also "You are a helpful" — identical tokens → identical hash → prefix hit. B does not recompute or allocate for page 0; it just borrows Step 4 — Request A gets retracted then resumed (memory pressure kicked it out, now it's back). On resume, A re-matches the tree. The scheduler sees page 0 is already the canonical This is the non-tail repoint: logical page 0 (NOT the tail) changed which physical box it points to, from Step 5 — A appends its next token (say it generates The bug, in tokensThe scheduler knows the correct row is Now When the attention kernel reads A's newest tokens' KV, it instead reads "You are a helpful"'s KV. Request A's generation silently drifts — even though sampling is greedy and the prompt never changed. That's exactly the deterministic-divergence symptom in the RFC. The fix:When required(based on full_refresh flag of the request), refresh/copy entire row ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 404f73dae5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
404f73d to
91ae189
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91ae189b32
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
91ae189 to
17663cb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dd87c97f8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
TL;DR: the req_to_page mirror was updated with only the appended tail, but some scheduler steps also repoint earlier non-tail pages, leaving stale or aliased row entries. Add a scheduler-produced full_refresh bit so the runtime can refresh the entire row when the authoritative scheduler row may have changed before the tail. The bit is set in scheduler/operations/forward.cpp at transitions that can rebuild, publish, adopt, or recover prefix pages: - SchedulePrefillFirstChunkEvent: the first chunk adopts/rebuilds the prefix from a fresh match, so the mirror must refresh the whole row rather than only the appended tail. - SchedulePrefillEvent: continuation prefill can publish/adopt completed prior-chunk pages before scheduling the next tail chunk, so earlier logical pages may be repointed even though the op also has a tail delta. - ScheduleDecodeEvent from PrefillDone: the transition can publish/adopt completed prefill pages before reserving the decode tail, so earlier logical pages may be repointed while the op still has a decode tail delta. - ScheduleDecodeFromRetractedEvent: resuming a retracted request re-matches and rebuilds the row from scratch; prefix pages can land on different physical pages plus a fresh tail. Pure decode continuation stays tail-only. Runtime update_block_table consumes full_refresh by replacing begin/size/new_occupied_pages with the scheduler's occupied_pages row from logical page 0, while preserving clamp behavior for oversized rows. Add focused tests for full-row vs tail selection, continuation-prefill prefix repoint, decode-tail plus prefix repoint, clamp on an oversized full-refresh row, and full_refresh overriding a zero tail size. Validation: - python -m pytest -q test/runtime/test_update_block_table_capacity_clamp.py - pre-commit run --all-files Signed-off-by: Stanley Winata <stanley.winata@amd.com>
Always copy updated req_to_page rows from forward_op.occupied_pages starting at logical page 0 instead of applying begin/size tail deltas. The scheduler owns the authoritative page row, and several prefix-cache transitions can repoint non-tail pages while still reporting a tail update; refreshing the whole row prevents stale or aliased mirror entries. Keep the existing capacity clamp for rows that exceed req_to_page width, but apply it to the authoritative row payload. Update the page-table tests to cover row-wide refresh, oversized-row clamping, and the zero-update early return. Validation: - python -m pytest -q test/runtime/test_update_block_table_capacity_clamp.py - pre-commit run --all-files Signed-off-by: Stanley Winata <stanley.winata@amd.com>
This reverts commit 2dd87c9. The always-refresh experiment did not improve local Kimi K2.5 MXFP4 AIME25 accuracy: both scheduler-driven full_refresh and row-wide refresh scored 0.75 on limit=4. Keep the scheduler-produced full_refresh mechanism as the targeted fix and avoid the broader per-step row refresh until we have a stronger signal. Restore the Kimi K2.5 MXFP4 AIME smoke threshold to 0.75, matching the observed stable score while this mirror fix is validated separately. Validation: - python -m pytest -q test/runtime/test_update_block_table_capacity_clamp.py - pre-commit run --all-files Signed-off-by: Stanley Winata <stanley.winata@amd.com>
Update the full-refresh block-table test to encode the concrete corruption mode: a non-tail prefix page is repointed to a canonical cached page while the freed physical page is reused as the new tail. A tail-only mirror update would leave logical page 0 and logical page 2 aliasing the same physical page. Keep the test CPU-only by asserting the update_block_table contract passed to the page-table kernel: full_refresh rows copy the scheduler's occupied_pages from logical page 0, while ordinary rows still use their tail delta. Validation: - python -m pytest -q test/runtime/test_update_block_table_capacity_clamp.py - pre-commit run --all-files Signed-off-by: Stanley Winata <stanley.winata@amd.com>
66f4193 to
959056d
Compare
Summary
TL;DR:
req_to_pageis a runtime mirror of the scheduler-owned logical page row. Updating that mirror with only the appended tail is unsafe when a scheduler transition also republishes, adopts, or rebuilds earlier non-tail pages. In those cases, the old mirror can keep stale physical page IDs and can even alias a recycled page into multiple logical slots.This PR adds a scheduler-produced
full_refreshbit. When the bit is set, runtime refreshes that request's wholereq_to_pagerow fromforward_op.occupied_pagesstarting at logical page 0. When the bit is not set, runtime keeps the cheaper tail-delta update.The bit is set in
tokenspeed-scheduler/csrc/scheduler/operations/forward.cppfor transitions that can change non-tail page identity:applyEventAndGenerateOp(SchedulePrefillFirstChunkEvent): the first chunk adopts/rebuilds the prefix from a fresh match, so the mirror must refresh the whole row rather than only the appended tail.applyEventAndGenerateOp(SchedulePrefillEvent): continuation prefill can publish/adopt completed prior-chunk pages before scheduling the next tail chunk, so earlier logical pages may be repointed even though the op also has a tail delta.applyEventAndGenerateOp(ScheduleDecodeEvent)when the request came fromPrefillDone: the transition can publish/adopt completed prefill pages before reserving the decode tail, so earlier logical pages may be repointed while the op still looks like a decode-tail update.applyEventAndGenerateOp(ScheduleDecodeFromRetractedEvent): resuming a retracted request re-matches and rebuilds the row from scratch; prefix pages are re-derived and can land on different physical pages, plus a fresh tail.Pure
Decoding -> Decodingremains tail-only and keepsfull_refresh=false.The runtime side consumes
full_refreshbefore its zero-size early return, so a full-refresh row is honored even if no new tail page was allocated. The existing capacity clamp is preserved for rows that exceedreq_to_pagewidth.The Kimi K2.5 MXFP4 AIME smoke threshold remains
0.75; local validation showed the scheduler-driven mirror fix did not raise that limit-4 score to1.0, so the PR should not turn that smoke into a perfect-score gate.Test Plan
pytest -q test/runtime/test_update_block_table_capacity_clamp.pypre-commit run --all-files