-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[V1] Enable prefill optimization for Gemma3n #22628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[V1] Enable prefill optimization for Gemma3n #22628
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant performance optimization for Gemma3n models by enabling a fast prefill path, inspired by the YOCO paper. The implementation is well-thought-out, involving a refactoring of the Gemma3n model into self-decoder and cross-decoder modules to work with torch.compile
and dynamic batch sizes. The changes to attention metadata and the use of a conditional compilation decorator are clean solutions. Overall, the changes are robust and the associated refactoring of KV cache sharing logic improves the codebase. However, there is a critical gap in testing that should be addressed.
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
This pull request has merge conflicts that must be resolved before it can be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution. I took a look on model runner changes. Will check gemma3m.py later.
a822572
to
bcf331a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almosst LGTM. But maybe we need more discussion about custom attention abstraction @LucasWilkinson
embed_scale: torch.Tensor, | ||
): | ||
super().__init__() | ||
self.decoder_layers = decoder_layers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will there be any hidden problem when the decoder_layers are registered with both Gemma3nTextModel.layers
and Gemma3nTextModel.self_decoder.decoder_layers
in nn.Module
? A cleaner solution would be to only register it in Gemma3nSelfDecoder
(but need to update the weight loader, can do it in a follow-up PR after the model structure is finalized)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, mostly did this for simplicity as I couldn't really think of a case where it would be problematic (though there could be). I do want to separate this to a separate PR if possible
This pull request has merge conflicts that must be resolved before it can be |
24ded7e
to
b6ee234
Compare
@heheda12345 addressed comments, ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for your patience for iterating on this optimization.
Some follow-ups:
- Abstract out changes in the model to make future model integration easier
- Register each weight only once in nn.Module.
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: Yong Hoon Shin <[email protected]>
Signed-off-by: Yong Hoon Shin <[email protected]>
Signed-off-by: Yong Hoon Shin <[email protected]>
Signed-off-by: Yong Hoon Shin <[email protected]>
Signed-off-by: Yong Hoon Shin <[email protected]>
Signed-off-by: Yong Hoon Shin <[email protected]>
Head branch was pushed to by a user without write access
d8dd6ca
to
e265fbd
Compare
I think the multi-modal processor test failure is indeed related to this PR. It has not been failing on the previous commits on main before this one |
@DarkLight1337 based on the commit history it does look like they started failing after my PR but the failing tests are:
and they were passing on my machine. Are you able to reproduce the issue on your dev machine? EDIT: my bad, seems like the tests only fail if they come after the Gemma3n test case. I was only testing only the two broken test cases and it was fine locally. After including Gemma3n test case, I can reproduce the issue. It's probably due to the weight loading issue, reverting the gemma3n changes in this PR fixes it. #23897 should unbreak the CI while I investigate the issue |
Btw, this has a bad interaction with PyTorch 2.8.0: #20358 I verified that after reverting this the failing test in CI (https://buildkite.com/vllm/ci/builds/28843/steps/canvas?jid=0198f5cd-d810-42bf-8560-c5ef36e6898c) passes |
I had an issue working with Lora adapters with the complication set at PIECEWISE, some changes seem relevant to the issues I'm facing #23970 |
@pratapyash could you check if you are still getting the issue after #23897? |
for those facing problems after this PR, please try rebasing on #23897 which reverts the gemma3n model changes |
Essential Elements of an Effective PR Description Checklist
supported_models.md
andexamples
for a new model.Purpose
This PR adds an option to enable prefill optimization for Gemma3n model with
--kv-sharing-fast-prefill
.Background
In You Only Cache Once (https://arxiv.org/abs/2405.05254), self-decoder layers generate KV caches while cross-decoder layers use cross-attention and reuse the shared KV cache. As only self-decoder layers generate KV caches, cross-decoder layers don't need to do prefill. Below is a figure from the YOCO paper on the prefill optimization:
Design
In vLLM V1, the scheduler does not distinguish between prefill and decode. Instead, tokens for requests doing prefill and decode are batched together, as illustrated below (source: vLLM blog):
When we skip tokens corresponding to prefill in the cross-decoder layers, we therefore will have the batch size reduced during model forward for the cross-decoder layers:
Without optimization enabled (baseline)
With optimization enabled (--kv-sharing-fast-prefill)
With this change, we can no longer compile the top-level model for 2 reasons:
Solution: we split the layers into self- and cross-decoder layers, and compile + graph capture them separately. For Gemma3n-E2B which has 30 layers, the first 20 layers and other 10 layers will be grouped separately into independently compiled and CUDA graph captured modules.
Other changes required in this PR:
make_kv_sharing_fast_prefill_common_attn_metadata
to create an attention metadata excluding all prefill tokens. This requires passinglogits_indices
toCommonAttentionMetadata
KVSharingFastPrefillAttentionMetadata
. This has two additional metadata (logits_indices_padded
andnum_logits_indices
) which are required for indexing into hidden states in the model implementation to match the shapes that the new attention metadata expectshidden_states
shape from[altup_num_inputs, num_tokens,hidden_size]
to[num_tokens,hidden_size, altup_num_inputs]
to ensurenum_tokens
(batch size) comes at dim 0. We cannot havenum_tokens
be on dim=1 because creating a slice along dim=1 would a) cause torch.compile tensor stride assertions to fail, and b) resolving this by callingcontiguous()
on the slice would cause memory copy and therefore violate CUDA graph static address constraint.--kv-sharing-fast-prefill
flag is passed, we take a differentself.fast_prefill_forward()
path which uses thelogits_indices_padded
metadata passed to index into the subset of tokens for cross-decoder layers (i.e. batch size is reduced). We then merge it back to the output of self-decoder to get the final output.--kv-sharing-fast-prefill
flag is passed, we will compile self-decoder and cross-decoder submodules separately, and we also need to pre-allocate static buffers for CUDA graph replay. If it is not passed (default), we will still compile the top-levelGemma3TextModel
Compared to trunk, the only difference is there are extra groups for attn_groups[0] and attn_groups[4] for layers which need a separate attention metadata builder for the fast prefill path. Previously it looks like this:
Important
When
prompt_logprobs
is enabled, we can no longer use fast prefill optimization. This is because by skipping all but last prefill tokens, the logits for the prompt tokens will no longer be valid. For example, multiple choice question (MCQ) evals useprompt_logprobs
to get the logprobs of continuation tokens (e.g. lm-evaluation-harness), so using--kv-sharing-fast-prefill
will yield inaccurate results. To prevent this, completion requests with prompt logprobs will be rejected (HTTP 400 Bad Request) for online serving. For offline serving (i.e. withLLM
class), it will raise an assertion error.Follow ups
Address IMA issue when using fast prefill with hybrid KV cache manager (currently hybrid KV manager is disabled if(EDIT: seems like [BugFix] Fix for IMA in FA3 varlen combine #22967 fixed it)--kv-sharing-fast-prefill
is set)self_decoder_hidden_state.clone()
in Gemma3nTest Plan
Evals
ran gsm8k, mmlu, mmlu pro
Unit tests
Performance
Perform sweep over
max-concurrency
andrandom-input-len
,$num_reqs = 256
max-num-batched-tokens = 8192
max-num-seqs = 128
Test Result
Evals
Evals on par
gsm8k: 0.5451 and mmlu_pro:0.3424 with hybrid memory allocator enabled as well
Unit tests
Unit tests all pass
Performance
Mean TTFT and TPOT (ms):