Skip to content

[WebGPU] Make max_num_pending_dispatches configurable#28894

Merged
hariharans29 merged 1 commit into
microsoft:mainfrom
Reranko05:webgpu-max-num-pending-dispatches
Jun 17, 2026
Merged

[WebGPU] Make max_num_pending_dispatches configurable#28894
hariharans29 merged 1 commit into
microsoft:mainfrom
Reranko05:webgpu-max-num-pending-dispatches

Conversation

@Reranko05

Copy link
Copy Markdown
Contributor

Description

Adds support for configuring the WebGPU execution provider's maximum number of pending dispatches via a provider option.

Previously, WebGpuContext::max_num_pending_dispatches_ was hardcoded to 16. This change introduces a new WebGPU provider option:

ep.webgpuexecutionprovider.maxNumPendingDispatches

The value is parsed into WebGpuContextConfig and applied during WebGpuContext initialization, while preserving the existing default value of 16.

A WebGPU execution test was also added to verify that the new provider option can be supplied successfully and that execution proceeds correctly with a custom value.

Testing

  • Built ONNX Runtime with WebGPU enabled.

  • Ran:

    • ExpandOpTest.Expand_3x3_int64_webgpu_max_num_pending_dispatches
    • ExpandOpTest.*webgpu*
  • All tests passed.

Motivation and Context

Fixes #28613.

Some devices may achieve better inference performance with a different pending dispatch limit. This change makes the existing hardcoded value configurable without changing the default behavior.

@Reranko05

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@Reranko05
Reranko05 force-pushed the webgpu-max-num-pending-dispatches branch from d49fc3a to 7df5bf7 Compare June 8, 2026 09:45
@qjia7
qjia7 requested a review from Copilot June 9, 2026 01:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes the WebGPU Execution Provider’s maximum number of pending dispatches configurable via a new provider option (ep.webgpuexecutionprovider.maxNumPendingDispatches), instead of being hardcoded to 16, while preserving the existing default behavior.

Changes:

  • Added a new WebGPU provider option key for maxNumPendingDispatches.
  • Parsed the option into WebGpuContextConfig and applied it during WebGpuContext one-time initialization (with a warning if a different value is later requested for an already-initialized context).
  • Added a WebGPU Expand test that supplies the new option and verifies execution succeeds with a custom value.

Reviewed changes

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

Show a summary per file
File Description
onnxruntime/test/providers/cpu/tensor/expand_test.cc Adds a WebGPU execution test that passes the new provider option.
onnxruntime/core/providers/webgpu/webgpu_provider_options.h Defines the new provider option key constant.
onnxruntime/core/providers/webgpu/webgpu_provider_factory.cc Parses maxNumPendingDispatches into WebGpuContextConfig and logs it.
onnxruntime/core/providers/webgpu/webgpu_context.h Extends WebGpuContextConfig and makes the context’s max pending dispatches configurable.
onnxruntime/core/providers/webgpu/webgpu_context.cc Applies the configured value during one-time context initialization and warns on mismatched reinit requests.

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

Comment thread onnxruntime/core/providers/webgpu/webgpu_provider_factory.cc
@Reranko05
Reranko05 force-pushed the webgpu-max-num-pending-dispatches branch from 7df5bf7 to 21efc5d Compare June 9, 2026 09:25
@qjia7
qjia7 requested a review from hariharans29 June 9, 2026 10:10
@qjia7

qjia7 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

The changes look good to me. Let’s see if Hari has any additional comments. Thanks.

@Reranko05
Reranko05 force-pushed the webgpu-max-num-pending-dispatches branch from 21efc5d to 6158924 Compare June 9, 2026 12:55
@Reranko05

Copy link
Copy Markdown
Contributor Author

Fixed the formatting issue reported by the lint checks.

@Reranko05

Copy link
Copy Markdown
Contributor Author

Hi @hariharans29, just a gentle ping on this PR when you have a chance.

@hariharans29

Copy link
Copy Markdown
Member

Review of PR #28894[WebGPU] Make max_num_pending_dispatches configurable

Verdict: approve with one small fix request. The feature itself is straightforward and well-motivated (closes #28613), @qjia7 already signed off, and the diff is appropriately surgical. There is one real concurrency bug in the warning path that should be fixed before merge — the cleanup also lets you delete a member you just added.


The fix

webgpu_context.cc:43-53 reads initialized_ and max_num_pending_dispatches_ outside std::call_once:

void WebGpuContext::Initialize(const WebGpuContextConfig& config) {
  if (initialized_ &&
      max_num_pending_dispatches_ !=
          config.max_num_pending_dispatches) {
    LOGS_DEFAULT(WARNING) << ...;
  }

  std::call_once(init_flag_, [this, &config]() {
    max_num_pending_dispatches_ = config.max_num_pending_dispatches;
    ...
    initialized_ = true;
  });
}

Both members are non-atomic, and concurrent calls to Initialize() are exactly what std::call_once is here to serialize. So during the window where Thread A is inside the call_once lambda writing max_num_pending_dispatches_/initialized_, Thread B can race into the pre-call_once check and read either value without synchronization. That's a data race per the C++ memory model and TSAN will flag it. On x86/ARM the torn-read risk for aligned bool/uint32_t is effectively zero, so this isn't a practical bug today, but it's the kind of thing that becomes a real one the next time someone refactors the surrounding code.

std::call_once already provides the synchronization you need. Move the comparison after it and you can also delete the initialized_ member entirely:

void WebGpuContext::Initialize(const WebGpuContextConfig& config) {
  std::call_once(init_flag_, [this, &config]() {
    max_num_pending_dispatches_ = config.max_num_pending_dispatches;
    ...
  });

  // call_once provides happens-before to subsequent reads of max_num_pending_dispatches_.
  if (max_num_pending_dispatches_ != config.max_num_pending_dispatches) {
    LOGS_DEFAULT(WARNING)
        << "WebGPU context is already initialized with maxNumPendingDispatches="
        << max_num_pending_dispatches_
        << ". Requested value " << config.max_num_pending_dispatches
        << " will be ignored.";
  }
}

Same observable behavior:

  • First call: lambda runs, sets the field, the post-call_once comparison trivially equals (requested == requested), no warning.
  • Subsequent call with the same value: lambda skipped, comparison equals, no warning.
  • Subsequent call with a different value: lambda skipped, comparison differs, warning fires.

The trade-off is that two callers concurrently passing matching non-default values won't see a warning — same as today. And you no longer need the bool initialized_ field in webgpu_context.h:353 (revert that line and the inner initialized_ = true; assignment).


Other observations

1. The = 16 member-initializer in webgpu_context.h:352 is now dead

After this PR, Initialize() always assigns max_num_pending_dispatches_ from config.max_num_pending_dispatches, which itself defaults to 16 in WebGpuContextConfig. The class-default is only reachable if Initialize() is never called, which would mean the context isn't usable anyway. Either drop the = 16 on the member or share a single constexpr uint32_t kDefaultMaxNumPendingDispatches = 16; between the two places so the default isn't duplicated. Minor.

2. 4096 upper cap

webgpu_provider_factory.cc:222:

ORT_ENFORCE(
    config.max_num_pending_dispatches <= 4096,
    "maxNumPendingDispatches must be less than or equal to 4096");

256× the default is a reasonable safety net. The comment says "to avoid excessive query buffer sizing" — fine. Worth a one-line note in the option docstring (in webgpu_provider_options.h) so users know the valid range without grepping the parser. Optional.

3. Validation coverage in tests

expand_test.cc:190-209 exercises a valid value ("32") and confirms the option doesn't break execution. There's no negative test for the new ORT_ENFORCE paths ("0", "4097", "not_a_number"). A small EXPECT_THROW test in the same file would lock in the contract — same pattern as the MlasBackendKernelSelectorRejectsInvalidKleidiAiConvIgemmMaxWork test that landed in #28571 a few days ago. Optional, but cheap insurance against accidental contract drift later.

4. Test does not verify the value was actually applied

The new test only verifies that supplying the option doesn't break Expand. It doesn't reach into the context to confirm max_num_pending_dispatches_ is actually 32. That's hard to do without exposing internals, so this is fine for now — the "did the option flow through the parser?" coverage is the realistic ask, and the test gets it.

5. The warning text reads cleanly

Mentions both the existing value and the ignored requested value. Good UX.


Things that look right

  • New option key follows the existing ep.webgpuexecutionprovider.* naming convention.
  • std::from_chars parsing with std::errc{} check on .ec is the right idiom — preferred over std::stoul/atoi for input validation.
  • Default is preserved (16), so this is purely opt-in for callers — no behavior change for anyone who doesn't set the option.
  • VERBOSE log line for the new value mirrors the surrounding context-config logging.
  • The 4096 cap is enforced before init, not after, so a misconfiguration fails fast rather than producing a silent oversized buffer.

Bottom line

Approve once the unsynchronized read of initialized_/max_num_pending_dispatches_ is hoisted past call_once (and the initialized_ member is dropped along with it). The cleanup is a net code-deletion on top of an already small PR.

@Reranko05
Reranko05 force-pushed the webgpu-max-num-pending-dispatches branch from 6158924 to 81d3409 Compare June 16, 2026 05:30
@Reranko05

Copy link
Copy Markdown
Contributor Author

@hariharans29 Thanks for the detailed review. Good catch on the synchronization issue around the pre-call_once check. I moved the conflicting configuration warning after std::call_once, removed the additional initialized_ state, and added a note documenting the valid range for maxNumPendingDispatches. I reran the WebGPU Expand tests locally and they all passed.

@hariharans29

Copy link
Copy Markdown
Member

Re-review — PR #28894

Verdict: approve. Commit 81d3409 addresses both substantive concerns from the prior pass exactly as requested.

What got fixed

  1. Concurrency / initialized_ removal. webgpu_context.cc:43-58 is now:

    void WebGpuContext::Initialize(const WebGpuContextConfig& config) {
      std::call_once(init_flag_, [this, &config]() {
        max_num_pending_dispatches_ = config.max_num_pending_dispatches;
        ...
      });
    
      if (max_num_pending_dispatches_ != config.max_num_pending_dispatches) {
        LOGS_DEFAULT(WARNING)
            << "WebGPU context is already initialized with "
            << "maxNumPendingDispatches=" << max_num_pending_dispatches_
            << ". Requested value " << config.max_num_pending_dispatches
            << " will be ignored.";
      }
    }

    The pre-call_once check is gone; the comparison now lives after std::call_once, which provides the happens-before that makes the read of max_num_pending_dispatches_ race-free. The bool initialized_ member is removed from webgpu_context.h, and the corresponding initialized_ = true; inside the lambda is gone. Same observable behavior, no data race, one fewer field. Exactly the shape suggested.

  2. maxNumPendingDispatches docstring. webgpu_provider_options.h:39-41 now has:

    // Valid range: 1-4096. Larger values are rejected to avoid excessive
    // query buffer sizing and unpredictable memory/performance behavior.
    constexpr const char* kMaxNumPendingDispatches = "ep.webgpuexecutionprovider.maxNumPendingDispatches";

    Users grep'ing for the option key now see the valid range without having to chase down webgpu_provider_factory.cc. Good.

What didn't change

The two optional items I flagged are still as-is, and that's fine:

  • The = 16 member-initializer in webgpu_context.h is still present. It's effectively dead (always overwritten by Initialize()), but harmless and not worth re-spinning CI for.
  • No negative-input EXPECT_THROW tests for "0"/"4097"/"abc". Optional from the start; the validation contract in the parser is straightforward enough that I won't push for it now.

Bottom line

Ship it. Net code reduction (+63/-1 vs the previous +66/-1) on top of an already small PR. Approve and merge once the remaining CI rebuild settles.

@hariharans29
hariharans29 requested a review from guschmue June 16, 2026 22:36
@hariharans29
hariharans29 enabled auto-merge (squash) June 16, 2026 23:30
@hariharans29
hariharans29 merged commit 57f6079 into microsoft:main Jun 17, 2026
85 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Performance] Proposal: Make max number of pending dispatches a WebGPU EP option

5 participants