Repository navigation
Conversation
a471ea3 to
8e345e9
Compare
Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
8e345e9 to
b5b0b78
Compare
Merge the cleaned Graph parent while preserving the scheduler changes and existing commit history. Remove benchmark and result files from the final PR diff. Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
Signed-off-by: 0z5a <Dezhen.lu@student.uni-tuebingen.de>
bjf-frz
changed the title
| if not self._restore_state(request, packet.state): | ||
| return False | ||
| if packet.rng_state is not None: | ||
| request.generator = torch.Generator(device=e.cache_manager.device) | ||
| request.generator.set_state(packet.rng_state) | ||
| e.scheduler.requests[request.request_id] = request | ||
| e.scheduler.enqueue(request, packet.state["stage"]) |
There was a problem hiding this comment.
Please route migration imports through scheduler admission before restoring KV.
_restore_state() only checks whether the snapshot’s current pages fit, and lines 170–171 then register the request directly as active. This bypasses max_num_seqs and the applicable KV growth budget.
I reproduced this with max_num_seqs=1, incremental KV allocation, preemption disabled, and a 32-block pool. Importing a second request returns True; both requests later exhaust the pool and fail with scheduler made no progress. The same workload completes through normal admission.
Please add a scheduler-owned admission operation for migration. If admission is temporarily blocked, return False without consuming the packet or leaving target-side state. Once admitted, restore the saved execution stage.
| def _signature(self): | ||
| e, cache = self.engine, self.engine.cache_manager | ||
| return ( | ||
| e.model.config, | ||
| cache.layout, | ||
| cache.block_size, | ||
| cache.dtype, | ||
| cache.key_cache.shape[1:], | ||
| cache.attention_info, | ||
| cache.device.type, | ||
| ) |
There was a problem hiding this comment.
Please validate the destination execution mode before allocating or restoring migration state.
_signature() checks model/cache compatibility but does not check whether the destination supports Stage.SPECULATIVE.
With the same model and cache configuration, a destination created without speculative_config accepts the packet and returns True. The request is then queued as SPECULATIVE, which the normal scheduling policy never handles; the next step() fails with scheduler made no progress.
Please reject this incompatible destination with a clear error before _restore_state(). Supporting migration into ordinary decoding would require an explicit state/stage conversion.
There was a problem hiding this comment.
Replace K with the number of speculative tokens to make the docstring clear without requiring readers to infer the notation.
|
The current benchmarks do not demonstrate a meaningful benefit from |
|
Please add benchmarks for equal-priority traffic, mixed-priority arrivals, mixed short and long requests, and low versus high load. If they show consistent TTFT improvements with only a small E2E penalty, enable preemption by default ( |
| allocation = cache._get_allocation(victim.request_id) | ||
| allocation = cache._get_allocation(request.request_id) | ||
| blocks = [b for table in allocation.block_tables for b in table] | ||
| # Copy views one page at a time: a pressure recovery must not allocate |
There was a problem hiding this comment.
Add a comment here explaining that copying KV pages individually to CPU avoids allocating a temporary GPU tensor to gather them before transfer.
| e = self.engine | ||
| e.model_runner.release(request.request_id) | ||
| e.cache_manager.poll_prefixes() | ||
| e.cache_manager.free(request.request_id) |
There was a problem hiding this comment.
Move the request cleanup logic in _release() into an engine method. Releasing runner state, freeing KV cache, removing queue entries, and clearing request tensors are engine responsibilities; PreemptionManager should delegate this cleanup to the engine.
|
Please remove the request migration feature and its related tests from this PR. A separate KV migration mechanism is unnecessary; the existing KV transfer infrastructure should be reused. Keep local preemption and resume support. |
|
resolve conflict please |
|
After making these changes, please run the |
Remove intra-round scheduling and request migration, reuse the current KV snapshot/restore interface, and move suspension cleanup into the engine. Merge current main to preserve the attention backend and admission changes. Add speculative preemption lifecycle/RNG regressions and paired traffic benchmarks with separate profiling. Keep preemption opt-in pending the requested performance evidence. Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
0z5a
changed the title
2 participants
Keep local priority preemption and resume at committed speculative-round boundaries. Reuse the existing KV snapshot/restore interface and move suspension resource cleanup into the engine. Remove intra-round scheduling and request migration, including their APIs and tests.
Resolve the conflicts against current main, retaining its attention backend and KV admission changes. Add regressions for preemption/resume, per-request sampling RNG, aborting suspended requests, and request-ID reuse.
Add a paired traffic benchmark and profiling recipe covering equal/mixed priority, uniform/mixed short-long requests, and low/high load. Preserve per-request TTFT/E2E, outputs, memory, and preemption counts; profile separately from timed measurements.
enable_preemptionremainsFalsepending evidence of consistent TTFT benefits with a small E2E penalty.Validation for this revision:
git diff --check: passed.rlt-perf-optprofiling: not run. The referenced skill/Developer Must-Read location has not been found in the available repository or skills.