[WebGPU] Zero-initialize writable device allocator buffers - #32063
[WebGPU] Zero-initialize writable device allocator buffers#32063Jiajia Qin (qjia7) wants to merge 14 commits into
Conversation
Encode zero-initialization directly when the device allocator reuses a cached buffer. Keep captured command storage dispatch-only while preserving the combined encoded and deferred dispatch limit.
There was a problem hiding this comment.
Pull request overview
This PR hardens the WebGPU EP’s device allocator by ensuring writable, model-visible device buffers are zero-initialized on allocation, preventing reuse of stale GPU memory contents across runs (addressing the info-disclosure risk described in #28974). It also adjusts WebGPU dispatch batching logic so allocator-triggered work draining doesn’t desynchronize profiling/pending-dispatch bookkeeping, and adds targeted regressions.
Changes:
- Add allocation-time zeroing for reused pooled device-allocator buffers via
BufferManager::Create(..., initialize_to_zero=true), while leaving read-only/initializer and other internal allocations unchanged. - Enable Dawn’s lazy clearing for fresh buffers by turning on the
lazy_clear_resource_on_first_usedevice toggle. - Tighten dispatch-window accounting (encoded + deferred) and add tests covering pooling, graph capture non-capture behavior, and profiled dispatch batching.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| onnxruntime/test/providers/webgpu/webgpu_context_test.cc | Adds tests for pooled-buffer zero init, graph-capture non-capture of allocator clears, and lazy-clear toggle enablement. |
| onnxruntime/test/providers/graph_capture_test.cc | Adds regression to ensure profiling/dispatch batching remains bounded across allocator clears. |
| onnxruntime/core/providers/webgpu/webgpu_context.cc | Bounds combined encoded+deferred dispatch count and enables Dawn lazy clearing via device toggles. |
| onnxruntime/core/providers/webgpu/buffer_manager.h | Extends BufferManager::Create signature with an initialize_to_zero parameter (default false). |
| onnxruntime/core/providers/webgpu/buffer_manager.cc | Clears reused cached buffers in command order when initialize_to_zero is set, with flush safeguards around pending dispatch limits. |
| onnxruntime/core/providers/webgpu/allocator.h | Adds allocator state to control allocation-time zero initialization. |
| onnxruntime/core/providers/webgpu/allocator.cc | Enables zero initialization for writable device allocator buffers and wires the flag into BufferManager::Create. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (initialize_to_zero) { | ||
| auto buffer_guard = wgpu::Buffer::Acquire(buffer); | ||
| ORT_THROW_IF_ERROR(context_.EncodeDeferredDispatches()); | ||
| if (context_.num_pending_dispatches_ >= context_.max_num_pending_dispatches_) { |
There was a problem hiding this comment.
Could we flush the command encoder after recording ClearBuffer() and before returning this buffer?
This path is also reachable through the public WebGPU shared allocator obtained via Env::GetSharedAllocator(). In that case, an ORT user can call Alloc() to create a device tensor and may expect the returned buffer to already be zero-initialized. Currently, ClearBuffer() is only recorded on ORT's command encoder, while Create() returns before the clear is submitted.
As a result, a caller using the returned buffer outside ORT's subsequent command sequence may observe that the buffer has not been cleared yet. Could we call context_.Flush(*this) here to establish the zero-initialization guarantee before returning?
I am not sure if the perf has regresion.
There was a problem hiding this comment.
Good catch. I think the perf impact is limited since it only impacts the writable device-allocator which usually are allocated in advance before the session.run. So I suppose it won't impact the run performance.
There was a problem hiding this comment.
Is this also used for intermediate tensors during model execution? Also, I noticed we're still using defer_size + dispatches_size elsewhere—do we still need that, or can it be removed?
There was a problem hiding this comment.
Yes, the session device allocator is also used for intermediate tensors. The latest implementation therefore avoids flushing for every allocation during Session::Run: the clear is encoded in command order and submitted by OnRunEnd(). Allocations outside a run, including the public shared allocator, submit the clear before Alloc() returns.
I also audited all GpuBufferAllocator construction sites. Read-only initializer allocators intentionally do not clear, while prepack allocations are submitted at the PrePack() boundary.
The combined num_pending_dispatches_ + deferred_dispatches_.size() check is still needed. These values represent already-encoded and not-yet-encoded dispatches respectively. Checking only the deferred count could exceed the dispatch/profiling window when encoded dispatches remain in the current command encoder. The allocation path itself no longer checks this limit; it flushes only when immediate zero-initialization submission is required.
Defer reused-buffer clear submission while a session run is active, while submitting immediately for allocations made outside Run and through the shared allocator. Track EP run state and cover both submission policies with allocator and graph-capture tests.
Include the concrete RunOptions definition and use explicit instances when exercising the execution provider run lifecycle. This fixes GCC compilation where RunOptions was otherwise incomplete.
|
Review — PR #32063: Zero-initialize writable WebGPU device allocator buffers Summary of changes (10 files, +317/−14) The core fix: when GpuBufferAllocator::Alloc is called on a writable device allocator and hands out a reused pooled buffer, BufferManager::Create now records Submission policy is a separate axis, driven by a new
Two additional hardening pieces travel with the fix:
Approach is sound and the security angle is real Issue #28974 is a legitimate GPU-memory-disclosure bug: a pooled buffer that held model activations gets recycled and shipped to a subsequent tensor whose consumer reads before writing. WebGPU's spec-level guarantee (fresh buffers zeroed on first use) doesn't help pooled buffers, because from Dawn's perspective they're not fresh. Explicit The split-policy (encode vs. submit) is elegant — I'd been half-expecting a Correctness checks I did
Comments / suggestions
Non-issues I verified
Recommendation Approve after (1) |
Description
BufferManager::Createin command order, and enable Dawn lazy resource clearing for fresh buffers.Session::Run, but submitted before allocation returns outsideRun; shared allocator clears are always submitted immediately.WebGpuContextand add regressions for allocation submission policy, graph capture, and profiled dispatch batching.Motivation and Context
Fixes #28974.
Reused device-allocator buffers can retain data from prior model execution. Writable session and shared allocations must therefore be zero-initialized when handed out. Reused buffers are explicitly cleared before use, while newly created buffers rely on Dawn's lazy-clear guarantee.
Clearing and submission are separate policies. During
Session::Run, the clear remains ordered in the current command encoder and is submitted with the surrounding dispatch batch. OutsideRun, including through the shared allocator exposed to users, the clear is submitted before allocation returns so subsequent queue work observes it.Allocator clears are allocation-time initialization rather than graph work. They are encoded in normal command order but are not stored in
CapturedCommandInfoor repeated byReplayGraph(), whose inputs and outputs are allocated in advance.Testing
onnxruntime_provider_testWebGpuContextTest.*WebGpuDispatchBatchingTests.*GraphCaptureTests.TestReleaseCapturedGraphInferenceSessionTests.TestGraphCaptureAll 16 focused tests pass.