[Gemma3] Reduce concatenate and copy in KV cache update to improve perf#2781
[Gemma3] Reduce concatenate and copy in KV cache update to improve perf#2781gaurides wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the cache mechanism in Gemma 3 models to use a tuple of per-layer (key, value) pairs instead of a stacked dense tensor, avoiding stacking overhead. The feedback highlights that the docstring for call_with_cache in gemma3_causal_lm.py should be updated to accurately reflect this new tuple-of-tuples structure instead of describing it as a dense float Tensor.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| x = text_embeddings | ||
|
|
||
| # Each decoder layer has a cache; we update them separately. | ||
| # Cache is a tuple of per-layer (key, value) pairs — no stacking. |
There was a problem hiding this comment.
The cache format has been successfully updated to a tuple of per-layer (key, value) pairs to avoid stacking overhead. However, please note that the docstring for call_with_cache (around line 148) still documents the cache argument as a dense float Tensor, the cache of key and value.. To prevent confusion for users or developers reading the API documentation, please update the docstring of call_with_cache to accurately describe the new tuple-of-tuples structure.
References
- Type information is provided in the docstring Args section using the format
arg_name: type. description. (link)
There was a problem hiding this comment.
Thanks for catching the docstring. I have pushed a commit to fix it.
|
Thanks for the detailed write-up. The optimization rationale is clear — eliminating A few things to address: Pattern divergence: Right now every model in the repo uses If this optimization is worth doing, we should adopt it as a repo-wide pattern, not a Gemma3-only special case. A one-off divergence means any future sampler or utility that assumes a tensor cache will break silently for Gemma3 only. GPU/TPU profiling: The profiling is from CPU. On GPU/TPU, XLA typically optimizes these stacks away via buffer reuse. Do you have numbers showing this helps on GPU/TPU too? Testing: The tuple cache format hasn't been used anywhere in the repo before, and it flows through |
|
Thanks @laxmareddyp for your comments; these are good points. I will follow-up on these. |
|
have you benchmarked the performance improvement? if yes, can you share it with us. |
|
@divyashreepathihalli : not yet, I was vacation. I will get back to it this week. |
Description of the change
This PR fixes issue #2780
When running Gemma3 on CPU, we observe significant % of time is spent on concatenate when doing KV cache update. This PR helps reduce time for KV Cache update and thus improve tokens/sec performance.
More details for the code change
The original code -- what it does and why it's slow
The cache is one big 6D tensor: [B, num_layers, 2, max_length, num_heads, head_dim]
During each token generation step, the flow is:
Steps 4 and 5 together account for the ~18% concatenate + ~10% copy_bitcast seen in the profile.
This solution -- what changes
The key idea: if the cache is never a single tensor, it never needs to be assembled into one.
Instead of one [B, 34, 2, S, H, D] tensor, the cache becomes:
The generation flow becomes:
Steps 4 and 5 are now free. Creating a Python tuple doesn't copy any tensor data -- it just creates a container holding references to the existing tensors.
Why does this work with JAX's while_loop?
JAX's while_loop (which the sampler uses) requires that loop-carried variables have the same "pytree structure" across iterations. A pytree is JAX's term for nested containers of tensors -- tuples, lists, dicts are all valid.
Our tuple of tuples:
is a valid pytree. Each individual tensor [B, S, H, D] maintains the same shape across iterations (only the content at position cache_update_index changes). So JAX is happy.
Summary of data movement per token
The slice_update copies remain (unavoidable -- attention needs the full K/V with the new token inserted), but the stacking copies are completely eliminated.
Colab Notebook
There is no change in API or model, the change is in implementation.
Checklist