Add StaticCache: fixed-capacity in-place KV cache for WebGPU - #1745
Add StaticCache: fixed-capacity in-place KV cache for WebGPU#1745kylo5aby wants to merge 2 commits into
Conversation
Signed-off-by: Zhenwei Jin <zhenwei.jin@intel.com>
nico-martin
left a comment
There was a problem hiding this comment.
Hi @kylo5aby, thank you so much for looking into this! The performance results are compelling and the overall approach makes sense. Before merging, we need to release per-step tensor wrappers, advance cache length only after successful inference, and make allocation transactional on failure. Could you also add focused lifecycle and WebGPU tests? I'm holding off on merging because the current failure paths can leak resources and corrupt cache state.
| const cachedTensors = new Set(Object.values(past_key_values)); | ||
| for (const tensor of Object.values(outputs)) { | ||
| for (const [name, tensor] of Object.entries(outputs)) { | ||
| if (is_static_cache && name.startsWith('present')) continue; |
There was a problem hiding this comment.
These are fresh ORT tensor wrappers each run even when they alias the static GPU buffers. Skipping disposal leaves per-step runtime handles to GC. Please explicitly release alias wrappers after each run while preserving the caller-owned buffers, and add a long-generation resource test.
There was a problem hiding this comment.
These are fresh ORT tensor wrappers each run even when they alias the static GPU buffers. Skipping disposal leaves per-step runtime handles to GC. Please explicitly release alias wrappers after each run while preserving the caller-owned buffers, and add a long-generation resource test.
Fixed, sessionRun() now reuses the caller's wrappers for pre-allocated outputs, so no per-step alias wrappers exist to leak, and the name-based dispose skip is gone. Covered by the sessionRun identity tests and a 300-step long-generation resource test.
| export async function runDecoderSession(session, inputs, past_key_values) { | ||
| if (past_key_values instanceof StaticCache) { | ||
| const num_new_tokens = (inputs.input_ids ?? inputs.inputs_embeds).dims[1]; | ||
| past_key_values._reserve(num_new_tokens); |
There was a problem hiding this comment.
This advances sequence length before sessionRun() succeeds. A rejected run leaves the cache logically ahead of its contents and breaks retries. Please commit length only after success or roll back on failure.
There was a problem hiding this comment.
This advances sequence length before
sessionRun()succeeds. A rejected run leaves the cache logically ahead of its contents and breaks retries. Please commit length only after success or roll back on failure.
Fixed, _reserve() is split into _checkCapacity() (before the run) and _commit() (only after the run succeeded and an in-place-write assertion passed). A failed run leaves the cache length untouched and retryable.
| } | ||
|
|
||
| for (const meta of session.inputMetadata) { | ||
| if (!cacheInputNames.has(meta.name)) continue; |
There was a problem hiding this comment.
If a later allocation throws, earlier buffers/properties remain while allocated is false, so retry allocates over partial state. Please allocate into local structures, commit atomically, and destroy temporary buffers on failure.
There was a problem hiding this comment.
If a later allocation throws, earlier buffers/properties remain while
allocatedis false, so retry allocates over partial state. Please allocate into local structures, commit atomically, and destroy temporary buffers on failure.
Fixed, allocation now goes into local structures and is committed atomically at the end; on failure all created wrappers are disposed and temporary GPU buffers destroyed before rethrowing, so the cache returns to a clean unallocated state and retry works
6f671c5 to
3ed80eb
Compare
Hi @nico-martin, thanks for the thorough review, all three issues were fixed, plus a few related hardening changes and a dedicated test suite. Added |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
nico-martin
left a comment
There was a problem hiding this comment.
Thanks for the updates! The wrapper cleanup and cache commit ordering look good now, and all 23 focused tests pass locally. There’s just one allocation edge case left: if createBuffer() succeeds but Tensor.fromGpuBuffer() throws, that new buffer never reaches the rollback logic and leaks. Could you destroy it inside createGpuBufferTensor() when wrapping fails and add a test for that path?
3ed80eb to
7727751
Compare
Signed-off-by: Zhenwei Jin <zhenwei.jin@intel.com>
7727751 to
fa41153
Compare
good catch, that buffer was indeed unreachable by |
nico-martin
left a comment
There was a problem hiding this comment.
Thanks for the updates! LGTM
Adds an opt-in
StaticCachethat eliminates per-step KV cache reallocation on WebGPU.Fixes #1741
With
DynamicCache(the default), every decode step replaces every KV tensor with a slightly larger one. On WebGPU each replacement destroys and re-creates a zero-filled GPU buffer, so the per-step cost grows with context length and jumps permanently at the buffer-pool bucket boundaries, producing a staircase-shaped latency curve.StaticCacheallocates each cache entry once at a fixedmax_cache_len, and binds the model'spresent.*outputs in place onto the same GPU buffers viasession.run(feeds, fetches). Decoding performs zero cache (re-)allocations per step.Usage
Performance
Decode throughput, q4f16 on WebGPU (TPS), 256 new tokens:
The gain grows with context length:
DynamicCachethroughput degrades as buffers get larger whileStaticCachestays nearly flat. Qwen3.5 (hybrid attention) benefits less because only its full-attention layers have growing KV entries. In our measurementsStaticCachematches a hand-written ORT static-KV baseline within 2%.Correctness
Token-identical output vs
DynamicCache(greedy, 256 new tokens, crossing the 2048/4096 bucket boundaries), on Phi-4-mini-instructand Qwen3.5-4B, Both paths are self-deterministic.Constraints
Enforced with explicit errors,
DynamicCacheremains the default everywhere:past_present_share_buffersemantics — true for GQA-based exports, which current WebGPU decoder exports use). This is why the feature is opt-in.