Enable adaptive verify window for ngram/draft speculation - #1023
Enable adaptive verify window for ngram/draft speculation#1023michaelneale wants to merge 1 commit into
Conversation
The adaptive verify window was never enabled on the split-serving path: to_embedded_openai_args hardcoded adaptive_speculative_window = false. With a fixed window, an early reject never shrank the window, so a sustained reject storm kept proposing at full depth and paying the full 2-round-trip recovery cost per token. On a WAN split this measured as ~40% throughput loss with N-gram speculation ON versus OFF, despite high per-token acceptance. Enable the adaptive window whenever speculation actually proposes a window (ngram or draft mode). The existing shrink_adaptive_window logic then narrows the window toward the observed accept depth after an early reject, cutting recovery frequency. Adds a regression test asserting ngram speculation turns the adaptive window on.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesSpeculative window behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Live A/B validation (2-node WAN split)Validated on a faithful 2-node split — M5 (Sydney, coordinator) + RTX 6000 Ada
The fix does what it claims: the adaptive window now actually engages Honest scopeThis is a correctness fix (the adaptive window should have been on for the Why: tolerating latency requires committing many tokens per round-trip. Recommendation: merge as a general correctness/robustness improvement (the |
Caveat: this makes WAN splits slightly worse — do not merge if WAN is the priorityWe care about WAN split serving, and the live A/B shows the adaptive window is a
The fix is mechanically correct (the window now adapts instead of being stuck), So: treat this as a latent correctness fix, not a WAN improvement. If anything, |
Follow-up: the real WAN cost is a redundant round-trip, not KV rollbackTraced the misprediction path in code. The KV rollback itself is cheap — a local It's the synchronization: on a reject the coordinator sends Implication for the next recovery fix (higher value than window shrinking): |
|
Superseded by #1026 |
What this fixes
N-gram speculation on a split runtime could be slower than no speculation
despite high per-token acceptance. On a 2-node WAN split (Sydney↔Melbourne,
~20ms RTT, MiniMax-M2.7) N-gram ON measured ~40% slower than OFF, with an 0.89
token accept rate but 231 early-reject windows each paying a 2-round-trip
recovery.
Root cause
The adaptive verify window was never enabled on the split-serving path:
to_embedded_openai_argshardcodedadaptive_speculative_window = false. Witha fixed window the window never shrank after an early reject
(
window_shrinks = 0in telemetry), so a sustained reject storm kept proposingat full depth and kept paying the full recovery cost per committed token. Over
WAN those extra round-trips dominate.
Change
Enable the adaptive window whenever speculation actually proposes a window
(
ngramordraftmode). The existingshrink_adaptive_windowlogic thennarrows the window toward the observed accept depth after an early reject,
cutting recovery frequency. Native-MTP-only decode is unaffected.
Adds a regression test asserting N-gram speculation turns the adaptive window
on.
Validation
cargo test -p mesh-llm-host-runtime --lib(new test + existing pass)cargo clippy -p mesh-llm-host-runtime --lib -- -D warningscargo fmt --all --checkwindow_shrinks > 0and throughput recovery).Summary by CodeRabbit
New Features
Bug Fixes
Tests