Fix the order of _free_encoder_inputs by gty111 · Pull Request #38907 · vllm-project/vllm

gty111 · 2026-04-03T10:40:22Z

Purpose

freeable can be triggered before fetch due to an inconsistency in how num_computed_tokens is updated.

After a request is scheduled, the scheduler immediately updates the token counter:

request.num_computed_tokens += num_scheduled_tokens

However, at this point the GPU has not actually finished computing those tokens yet. Some of these tokens are still in the scheduling pipeline and have not been executed.

Later, in _free_encoder_inputs, the logic uses the updated num_computed_tokens to determine whether the encoder inputs are freeable. Since the counter already includes tokens that are only scheduled (but not yet computed), the condition may incorrectly evaluate to true.

As a result, encoder inputs may be marked as freeable too early, even though the corresponding GPU computation has not completed.

Test Plan

Test Result

Essential Elements of an Effective PR Description Checklist

The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
The test plan, such as providing test command.
The test results, such as pasting the results comparison before and after, or e2e results
(Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
(Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: Tianyu Guo <guoty9@mail2.sysu.edu.cn>

Copilot

Pull request overview

This PR addresses a scheduler/model-runner ordering issue where encoder inputs could be marked freeable (and subsequently evicted) based on scheduled tokens rather than actually processed tokens, leading to encoder cache misses under high concurrency in multimodal + speculative decoding workloads.

Changes:

Reorders Scheduler._update_after_schedule so _free_encoder_inputs runs before request.num_computed_tokens is advanced for newly scheduled tokens.
Adds several debug print(...) statements across scheduler, encoder cache manager, and GPU model runner paths (currently not gated).

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

File	Description
`vllm/v1/core/sched/scheduler.py`	Calls `_free_encoder_inputs` before advancing `num_computed_tokens` to prevent premature encoder-input freeing.
`vllm/v1/worker/gpu_model_runner.py`	Adds stdout debug prints around freeing/fetching encoder cache entries.
`vllm/v1/core/encoder_cache_manager.py`	Adds stdout debug prints when items become freeable/are evicted.

Comments suppressed due to low confidence (1)

vllm/v1/core/sched/scheduler.py:1659

Remove the debug print here (or gate it behind the project's logger at an appropriate level). This runs in the scheduling hot path and will spam stdout / degrade performance under concurrency.

            start_pos = mm_feature.mm_position.offset
            num_tokens = mm_feature.mm_position.length
            if self.is_encoder_decoder and request.num_computed_tokens > 0:
                # With Whisper, as soon as we've generated a single token,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-03T10:43:39Z

        for req_id, num_scheduled_token in num_scheduled_tokens.items():
            request = self.requests[req_id]
-            request.num_computed_tokens += num_scheduled_token
-            request.is_prefill_chunk = request.num_computed_tokens < (
-                request.num_tokens + request.num_output_placeholders
-            )
-            scheduler_output.has_structured_output_requests |= (
-                request.use_structured_output and not request.is_prefill_chunk
-            )
-
            # NOTE: _free_encoder_inputs relies on num_computed_tokens, which
            # may be updated again in _update_from_output for speculative
            # decoding. However, it is safe to call the method here because
            # encoder inputs are always part of the prompt, not the output,
            # and thus are unaffected by speculative decoding.
            if request.has_encoder_inputs:
                self._free_encoder_inputs(request)
+            request.num_computed_tokens += num_scheduled_token
+            request.is_prefill_chunk = request.num_computed_tokens < (
+                request.num_tokens + request.num_output_placeholders
+            )
+            scheduler_output.has_structured_output_requests |= (
+                request.use_structured_output and not request.is_prefill_chunk
+            )


This change fixes a subtle ordering bug between _free_encoder_inputs and num_computed_tokens advancement. Please add a focused unit/regression test (likely in tests/v1/core/test_scheduler.py) that reproduces the premature-free scenario (scheduled-but-not-computed tokens causing an encoder cache entry to become freeable/evictable) and verifies it no longer happens.

gemini-code-assist

Code Review

This pull request reorders the scheduler logic to call '_free_encoder_inputs' before updating the computed token count for a request. It also introduces several debug print statements across the cache manager, scheduler, and model runner. The review feedback identifies these print statements as items that should be removed before merging to avoid log clutter and potential performance impacts.

I am having trouble creating individual review comments. Click here to see my feedback.