feat(rollout): add continuous batching generation prototype - #8368
Conversation
Signed-off-by: nathon-lee <leejianwoo@gmail.com>
Signed-off-by: nathon-lee <leejianwoo@gmail.com>
Signed-off-by: nathon-lee <leejianwoo@gmail.com>
Signed-off-by: nathon-lee <leejianwoo@gmail.com>
Signed-off-by: nathon-lee <leejianwoo@gmail.com>
Signed-off-by: nathon-lee <leejianwoo@gmail.com>
There was a problem hiding this comment.
Hi Nathon, from my understanding, your design would assume the decode front is on the same column. So this is why in the draft prompts need to have equal length. And when one row hit EOS, you will fill another prompt and make sure the second token after prefill will align with the decode front.
If this understanding is correct, then I have two suggestions:
-
sort the prompt from longest to shortest, right align prompts in the first batch then prefill in a way that the decoding front would be aligned. And when one row hit EOS and you want to fill another prompt in, there is always enough space to put the (shorter) new prompt kv in a way that the second token after prefill will align with the decode front.
-
periodically looking for 'dead zone'(columns before the earliest active row's span start) in the beginning and trim them by shift the rest tokens to the left. The trim would always be safe because newly added prompt will be shorter than active prompts in all rows, so there is always space to add new prompt after trim. By keeping trimming, we don't need to maintain a very large buffer for continuous batching and make it GPU memory size friendly (the length we needed for buffer is max(prompt_len + max_new_tokens)).
These suggestions assume we are not in serving scenario, where samples arrive in random order and serve in FIFO manner. Because it is rollout, we don't necessarily honor generation order in the batch (no TTFT and TPOT SLA), this is the condition we should exploit.
|
Thanks for the detailed suggestions, @delock. Your understanding is correct. I agree that sorting and right-aligning the prompts, together with trimming dead zones, could improve decode-front alignment and reduce the required buffer size. I’ll investigate these ideas in a follow-up and keep this PR focused on the baseline prototype. |
I saw your PR is still in draft state. Let me know when you finished your change and ready for review. Thanks! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67b246ef19
ℹ️ 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".
Signed-off-by: nathon-lee <leejianwoo@gmail.com>
|
Sorry for the delayed response. I was traveling for work recently. Thanks for your patience. Thanks, @delock. I agree that
The continuous path will retain the current prototype limitations. If this matches your expectation, I’ll update the implementation, tests, and documentation accordingly. |
Route continuous batching through generate() with a shared SamplingConfig and batch-level max_new_tokens budget. Return an ordered RolloutBatch, hide scheduler details from public exports, and update the rollout tests and documentation. Signed-off-by: nathon-lee <leejianwoo@gmail.com>
|
Hi Nathon, the interface looks good. I'll start review your implementation.
|
There was a problem hiding this comment.
Thanks @nathon-lee — the unified interface looks right, and the rework faithfully implements what we settled in the interface thread: single generate() entry point, SamplingConfig.continuous_batch_size: Optional[int] = None, a single right-padded RolloutBatch in original row order, and the scheduler types moved out of the public exports. I verified the core bookkeeping (row compaction, per-row write positions, prefill copies, logical positions) end-to-end on CPU and found no regressions; the full unit suite passes locally as well.
Remaining items before merge — 2 blocking, 5 minor. Details inline. Two known gaps at the generate() dispatch boundary (use_shared_prefill + CB is silently ignored; enable_profiling + CB produces no profile) are near-harmless today since CB requires n_samples_per_prompt=1 — can you open open follow-up issues covering both? They won't hold this PR.
[B2] No test coverage for the modern (StaticCache) path — _supports_cache_class = True never appears in the test suite. The only end-to-end test exercises the legacy path, while real models run the modern one, so our flagship path currently has no CI coverage. This is testable deterministically on CPU: a fake cache-class model whose next token depends on the row's cache contents (e.g. logits derived from the row's cached-value sum) makes any write-position / compaction / prefill-copy error change the outputs. Happy to share a reference implementation.
[B3] cache.compact runs unconditionally every decode step — steady-state steps pass an identity permutation and still pay a full-KV index_select + clone + copy (GB-scale per step at production sizes), and tail retirements leave the prefix already correct. Suggested fix, three layers:
- skip entirely when
update.retiredis empty; - inside
_compact_rows, skip the survivor copy whenactive_indices == arange(count), but keep the zeroing of dead rows — row reuse depends on it (a stale attention-mask row would make the next occupant attend to the previous request's KV); - apply the same treatment to the attention-mask compaction at the call site.
In-place refill (admitted row = retired row, no survivor movement on 1:1 churn) can be a follow-up.
Scope of CB with legacy model
I believe the legacy CB path should be removed. For these reasons:
- OPSD rollouts target modern reasoning models, all of which support cache classes;
- the fallback rebuilds the full cache every step — running a throughput feature on the slowest possible cache management undermines its purpose;
- net deletion of ~120 lines plus their dedicated tests;
@PKUWZP @minjiazhang for this opinion
Concretely: raise a clear ValueError for models without _supports_cache_class ("continuous batching requires a model with cache-class support; use the default generate() path or upgrade transformers"). The goal is to keep unnecessary complexity out. If we decide to keep it, one small fix applies (legacy path should reuse the incoming RolloutRequest instead of re-catting rows).
| rollout.generate(request, SamplingConfig(max_new_tokens=2, temperature=0, continuous_batch_size=1)) | ||
|
|
||
|
|
||
| def test_continuous_generation_refills_legacy_cache_batch(): |
There was a problem hiding this comment.
This is our only CB end-to-end test and it exercises the legacy path (_supports_cache_class = False). The modern path — the one every real model takes — has no coverage. Could you add a CPU e2e in the same style, with a fake cache-class model that calls past_key_values.update(...) per layer and derives logits from the row's cached values? That pins down the KV bookkeeping (write positions, compaction, prefill copies) deterministically.
| keep_slots = torch.tensor(update.keep_slots, dtype=torch.long, device=device) | ||
| survivor_count = keep_slots.numel() | ||
| if survivor_count: | ||
| cache.compact(keep_slots) |
There was a problem hiding this comment.
This runs on every decode step. With no retirement keep_slots is the identity permutation, so we pay a full-KV copy for nothing; with tail-only retirements the prefix is already correct and only the dead rows need clearing. Suggested guard structure:
if update.retired:
cache.compact(keep_slots) # + identity fast path inside _compact_rows (keep the zeroing!)Important detail: the identity fast path must skip only the survivor copy — the zeroing of rows [count:] is what makes retired rows safe to reuse (a stale mask row would corrupt the next occupant). The attention-mask compaction below deserves the same treatment.
| self.keys.zero_() | ||
| self.values.zero_() | ||
|
|
||
| def compact(self, active_indices: torch.Tensor) -> None: |
There was a problem hiding this comment.
DeepSpeedStaticLayer.compact has no callers in production or tests — DeepSpeedStaticCache.compact (via layer._compact_rows) is the only entry point. Please remove it; this also eliminates the duplicated validation block.
| target_layer.values[target_row, :, cache_start:cache_position].copy_(prefill_layer.values[source_row]) | ||
| for source_row, target_row in enumerate(update.admitted_slots): | ||
| attention_mask[target_row, cache_start:cache_position].copy_(prompt_attention[source_row]) | ||
| write_positions = cache._write_position |
There was a problem hiding this comment.
The rollout already owns this tensor (created in _generate_continuous, registered via set_write_position), so reaching into the cache's private attribute is avoidable — please thread it as a parameter like attention_mask already is.
Remove the corporate copyright line from the new rollout module and tests to match the existing SPDX and DeepSpeed Team header format. Signed-off-by: nathon-lee <leejianwoo@gmail.com>
|
Thanks, @delock, for the thorough review and CPU validation. The B2/B3 issues and cleanup suggestions are clear. I’ll add modern StaticCache CPU E2E coverage, optimize compaction while preserving dead-row zeroing, remove the unused/private implementation details, and update the experimental wording. I’ll also open follow-up issues for the shared-prefill and profiling dispatch behavior. For the legacy CB path, I agree with the rationale and will confirm with PKUWZP and minjiazhang before removing it. |
Add CPU coverage for the modern StaticCache continuous-batching path and avoid unnecessary cache and attention-mask compaction during steady-state decode. Remove the unsupported legacy continuous-batching fallback, clean up unused cache APIs, and update the experimental documentation. Signed-off-by: nathon-lee <leejianwoo@gmail.com>
| while update.active: | ||
| keep_slots = torch.tensor(update.keep_slots, dtype=torch.long, device=device) | ||
| survivor_count = keep_slots.numel() | ||
| if survivor_count: |
There was a problem hiding this comment.
Here cache reset only happens when all rows retire at the same time step. This will likely happen when all rows reaches max new tokens. However if some row retire with EOS (this is why continuous batching is needed), then reset may never happen and we will eventually hit the end of the cache with error.
Trim and shift the whole cache to the left could address this issue. Check periodically if at least N columns in the front of cache does not contain survivors cache, these columns can be removed by moving kv cache elements to the left, and free up the right part of the cache for further decoding. Update decode position accordingly. Pick a proper N could make balance between trim overhead and extra memory space needed.
There was a problem hiding this comment.
Thanks, @delock. You’re right that the current reset only happens when all active rows retire at the same step. With staggered EOS and continuous refill, the cache position can keep increasing until it exceeds the allocated cache length.
I’ll add periodic dead-prefix trimming for the modern StaticCache path, update the attention mask, write positions, and decode position after the shift, and add a deterministic CPU regression with staggered EOS/refill that exceeds the original cache span. I’ll keep the trimming threshold internal and avoid changing the public API.
Trim unused cache prefixes during continuous batching so staggered EOS and request refill cannot exhaust the allocated StaticCache span. Update the cache position, write positions, and attention mask after trimming, and add CPU coverage for staggered EOS refill and cache shifting. Signed-off-by: nathon-lee <leejianwoo@gmail.com>
|
@nathon-lee Can you fix formatting error? Thanks! |
Signed-off-by: nathon-lee <leejianwoo@gmail.com>
OFC, I’ll fix the formatting and push an update shortly. |
Summary
Add an experimental continuous-batching generation path to DeepSpeed rollout for OPSD/OPD evaluation.
The implementation maintains a bounded number of active request rows, retires requests independently on EOS or when the shared generation budget is reached, compacts surviving
StaticCacherows when necessary, and refills released slots with pending requests.The existing
HybridEngineRollout.generate()behavior remains unchanged when continuous batching is not enabled.What changed
SamplingConfig.continuous_batch_sizeto enable continuous batching through the existinggenerate(request, sampling)entry point.RolloutBatch.DeepSpeedStaticCachewith per-row write positions and active-row compaction.StaticCachepath.Design
Continuous batching is enabled with:
When enabled:
Nrequest rows remain active at a time.RolloutBatchis restored to the original request-row order.The scheduler is kept separate from the model backend. The rollout backend owns prompt prefill, decode execution, cache movement, and attention metadata construction.
Scope and limitations
This is an experimental rollout feature intended for OPSD/OPD evaluation.
The current implementation supports:
The following are intentionally left for follow-up work:
Models without Transformers cache-class support continue to use the default
generate()path.Validation
Focused rollout tests:
Focused H200 validation:
52 passed, 2 warnings
Coverage includes:
StaticCacheprefill and decode.