Repository navigation
Pin unslothai#269 (server-side preemption and exact concurrency) at 18365c117 - #211
danielhanchen wants to merge 46 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
#210's branch carries an old ggml-org#25731 and is refused on newer tags, so the change set is pinned from #260, a branch on upstream b11540 (1e6f04a) that composes with the Inkling and DiffusionGemma pins additively. Adds the scheduling fixes (an aged head is served, any resident rotates, the cheapest covering victim parks) and the /props preempt report.
danielhanchen
changed the title
# Conflicts: # scripts/unsloth/pr-set.json
…(b11540) #260 on b11540 no longer merges into the nightly tags (b11577 onward conflict in common/common.cpp, src/llama-kv-cache.cpp and tests/CMakeLists.txt). #269 carries the same change set on b11577 plus fixes for the RPC page-table refusal, the event query ABI and exact-mode restore validation.
# Conflicts: # scripts/unsloth/feature-checks.json # scripts/unsloth/pr-set.json
danielhanchen
changed the title
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Adds the review fixes since b8f7b90: exact mode refuses every host-placed weight outside the input layer, ty-clean preemption tests, the HIP VGPR ignore entry for the paged vec kernel, steadier timing tests, and documentation of the page table and the parking RAM gauge.
danielhanchen
changed the title
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…emption-197 # Conflicts: # scripts/unsloth/feature-checks.json # scripts/unsloth/pr-set.json
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…emption-197 # Conflicts: # scripts/unsloth/pr-set.json
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
3 similar comments
|
@codex review |
|
@codex review |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…pin/server-side-preemption-197 # Conflicts: # scripts/unsloth/feature-checks.json # scripts/unsloth/pr-set.json
…pin/server-side-preemption-197 # Conflicts: # scripts/unsloth/feature-checks.json # scripts/unsloth/pr-set.json
danielhanchen
changed the title
# Conflicts: # scripts/unsloth/pr-set.json
…pr211_repin_c732 # Conflicts: # scripts/unsloth/pr-set.json
Update (2026-10-23): #269 at c732f0f
scripts/unsloth/pr-set.jsonnow pins #269 at c732f0f, its head after 21 review rounds since 18365c1 (the last ones approved 4/4), refreshed onto upstream b11838 (4231b23), the base the prebuilt now picks. The pin moved one line; the other pins are master's, unchanged here.Pin-set simulation of this branch's set on b11838: #269 merges additively and its pin contract holds. Four of master's other pins are refused there: ggml-org#25731 (Inkling), #243, #266 and #314 (Flint). #318 repins or drops ggml-org#25731, #243 and #314; they are outside this PR, which only moves the #269 pin.
Update (2026-10-21, later): master merged in, #269 at 18365c1
Master is merged in (#312: Inkling at e1b89cf, #304 IQ1_M MMQ, #311 Beam, #314 Flint), so the set is master's 23 pins plus #269, which now pins 18365c1 (#269 merged b11793: upstream ggml-org#30679 changed the
gguf_kv_to_strdeclaration insrc/llama-impl.hbeside #269's lines; #269's own diff is unchanged per file).verify --cuda on b11786 (B200): CPU gate build ok, feature matrix 15 of 15 on CPU, full CUDA build ok, test-chat pass, test-backend-ops MUL_MAT 2606/2606, MUL_MAT_ID 1077/1077, PAD 37/37, GET_ROWS 349/349, feature matrix 15 of 15 on GPU.
#269's CUDA preemption suites at 18365c1 on a Colab L4: 88 passed, 0 failed, 10 skipped, the same per test as at 15b4e3b.
Update (2026-10-21): #269 at 15b4e3b, ggml-org#25731 at e1b89cf
scripts/unsloth/pr-set.jsonnow pins #269 at 15b4e3b, which merges upstream b11780 and b11782 and never picks a TTS or realtime transcription slot to park. It also repins ggml-org#25731 (Inkling) to e1b89cf, the same repin as #310, because 534c321 no longer merges on any base from b11760 on.The b11793 refusal comes from upstream ggml-org#30679 (824eaec, vocabulary loading), the only commit in b11792..b11793 touching those files. #269 will take b11793 before the aged base reaches it.
verify --cuda on b11785 (B200): CPU gate build ok, feature matrix 13 of 13 on CPU, full CUDA build ok, test-chat pass, test-backend-ops MUL_MAT 2508/2508, MUL_MAT_ID 1043/1043, PAD 37/37, GET_ROWS 349/349, feature matrix 13 of 13 on GPU.
Pins server-side preemption from #269 at f57ea1b. The old #260 pin sat on upstream b11540, was refused on the current nightly tags and is no longer in the set. #269 carries the same change set; its base branch is now
base/upstream-2639a3b4e(b11642), so its diff (+8.6k/-0.2k, 81 files) is exactly what gets pinned, and it composes with the other pins additively.The prebuilt is upstream's aged
b####tag plus the pins inscripts/unsloth/pr-set.json, merged in order (git diff3, thenadditive_merge.pyfor pure additions only). #269 had to take b11612 itself: upstream ggml-org#30023 (first in b11596) foldedtests/test-state-restore-fragmented.cpp, which #269 extends, intotest-llama-context, so #269 at f032338 was refused on every tag the nightly can now pick. #269 merged b11612 and moved its check into upstream's newfragcase. It then took b11641 as well: upstream ggml-org#30506 (LiliCorr drafting) edits the DFlash block size on the line where #269 bounds it by the caller's remaining context, so #269 at ce0965c was refused on b11641; and ggml-org#30498 in b11642 sits beside its options incommon/common.hand its README rows, so it took b11642 too.What #269 adds over #260, besides the port:
--preempt-policy fcfs|cost(LLAMA_ARG_PREEMPT_POLICY), defaultfcfs: the newest running request is parked and parked requests resume oldest first, the order vLLM's scheduler uses.costkeeps the smallest-covering-victim choice.-c 8192 -np 4), a rotated-out slot queues behind the request it made room for, re-prefills keep their reservation,n_cmpl > 1overflows no longer error every slot, in-flight restores land before a last-resort park.n_cmpl > 1prompt is no longer held behind the request it parked (fcfs deadlock), the rotation margin is charged only on what stays resident, the transfer fence is recorded when a transfer is issued rather than after every decode, and caches whose sequences do not share cells (DeepSeek V4, recurrent) are never parked (llama_memory_shares_cells()).--preempt-ramis restored where it had drifted: the non-causal decision check, the context shift left to the decode, and nopreemptrecord in responses.supports_oprefuses pagedFLASH_ATTN_EXTshapes the paged kernel does not run.FLASH_ATTN_EXT; the event query is published throughget_proc_address, soggml_backend_device_istays upstream's.Every preemption path is still off by default:
--preempt-ramdefaults to 0, so a server launched without it behaves like upstream, and exact concurrency is an environment opt-in. Studio passes--preempt-ramwhen it wants the server to park.Also merges master (the #276 repin onto b11609), and records #269 under
uncheckedin feature-checks.json in place of #260.Composes with master (2ca675f, 2026-10-16)
Master merged again (#282: model arch fix pins, ggml-org#30670 for Beam, and the now-upstream ggml-org#30520 dropped). #269 at f57ea1b needed no change. The 19 pins compose on every tag from b11681 (aged) to b11696:
CPU verify on b11681 and b11696: gate build ok, feature matrix 11 of 11 on CPU. The b11681 merged tree, rebuilt on a Kaggle T4x2 with the nightly's merge steps (tree hash feb3976bc equal to the local sim), CUDA sm_75:
Composes with master (706f461)
Master merged in (#273's #243 repin, #278's ggml-org#25731 repin and ggml-org#30520, #280's ggml-org#30616, #264, #266); #269 is appended last. ggml-org#30520 rewrites the LiliCorr walk's loop header, right above a line #269 adds, so #269 at 3ef8b53 was refused on every tag; #269 at 3a68a5c moves that line clear of the header. ggml-org#30580 (b11669) also added a single-string
override_kvto the server test helper beside #269's list of the same name; #269 at f57ea1b renames its list. With it, the 20 pins (master's 19 plus #269) compose on every tag the nightly can pick, b11669 included:Verify on b11658 (aged) and b11669 (newest), CUDA on a B200 (run with #269 at 3a68a5c; f57ea1b changes only a test helper, and the sim above was rerun on it):
#269's preemption suites on the merged b11669 server (CUDA build, B200): 85 passed, 8 skipped; the non-causal model test passes after f57ea1b, and the one CPU-only parked-head test passes in 42 s on the merged tree's CPU build with the suite's thread cap (it timed out uncapped at load 500).
Earlier runs, on older heads of #269:
Verify on b11642 (newest), CPU:
The same set on b11641 with #269 at 49b2552, rebuilt on a Kaggle T4 with the nightly's merge steps (tree hash equal to the local sim), CUDA sm_75:
#269's own suites on d1766cc, CPU:
test_preempt.py65 passed, 14 skipped under fcfs and under cost,test_preempt_models.py+test_preempt_notify.py17,test-llama-context130 of 130. On a Kaggle T4 (sm_75) at d1766cc, CUDA:test_preempt.py71 passed, 8 skipped under each policy with the async park path running,test_preempt_models.py+test_preempt_notify.py17,test-event-queryandtest-state-seq-copypass. On AMD gfx1151 (ROCm/HIP) at 37254a7, four chats that overflow the shared pool finish with parks where b11612 fails all four with context errors.Known, not from this pin
On b11617 a B200 failed 5
MUL_MAT_IDcases that also failed with no pins; on b11658 and b11669 all 1043 pass.Not run
Vulkan, Metal, Windows and ARM legs of the merged prebuilt run only in the nightly; the ROCm run above is #269 alone, not the merged set. The verify above is on a B200 (sm_100); the older-GPU runs (T4, L4, A100) are on earlier heads of #269.