Skip to content
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

fix(ext/webgpu): Create GPUQuerySet converter before usage #26883

Merged
merged 1 commit into from
Nov 17, 2024

Conversation

reczkok
Copy link
Contributor

@reczkok reczkok commented Nov 15, 2024

Hi, while trying to do performance tests for TypeGPU we encountered a problem when using Timestamp Queries.

Converter for GPUComputePassTimestampWrites uses converter for GPUQuerySet before it is defined making it impossible to use. This PR simply reorders converter creation to resolve this issue. Logging the GPUQuerySet still fails (as mentioned in #26769) but it is now usable inside pass descriptors and works when used.

@CLAassistant
Copy link

CLAassistant commented Nov 15, 2024

CLA assistant check
All committers have signed the CLA.

@littledivy littledivy merged commit 80098bf into denoland:main Nov 17, 2024
17 checks passed
@jowens
Copy link

jowens commented Nov 19, 2024

@reczkok With this fix, you are getting valid timestamps in a WebGPU program? With your fix, I am not, but I'm on a M3 MacBook Air, and there are other issues (generally) with timestamps on M3.

jowens pushed a commit to jowens/webgpu-sandbox that referenced this pull request Nov 19, 2024
@jowens
Copy link

jowens commented Nov 19, 2024

Repro here: https://github.com/jowens/webgpu-sandbox/blob/main/deno-standalone-timing-mre.mjs

% ~/Documents/src/deno/target/release/deno run deno-standalone-timing-mre.mjs
workgroup count: 262144
      workgroup size: 64
      maxComputeWGPerDim: 65535
      dispatchGeometry: 32768,8
Memdest size: 16777216 | No errors!
Timing raw data: 0n 0n 0
Timing result: 0 ns; transferred 134217728 bytes; bandwidth = Infinity GB/s
% ~/Documents/src/deno/target/release/deno --version
deno 2.0.6 (stable, release, aarch64-apple-darwin)
v8 13.0.245.12-rusty
typescript 5.6.2

(built today from top-of-tree)

@reczkok
Copy link
Contributor Author

reczkok commented Nov 19, 2024

Hi @jowens! I just tested the code in your repo and it seems to work just fine for me.

% ../DenoFork/deno/target/debug/deno run main.ts  
workgroup count: 262144
        workgroup size: 64
        maxComputeWGPerDim: 65535
        dispatchGeometry: 32768,8
Memdest size: 16777216 | No errors!
Timing raw data: 5747615510208n 5747616430958n 920750
Timing result: 920750 ns; transferred 134217728 bytes; bandwidth = 145.77000054303556 GB/s

I'm on a MacBook Pro M1

% ../DenoFork/deno/target/debug/deno --version  
deno 2.0.6 (stable, debug, aarch64-apple-darwin)
v8 13.0.245.12-rusty
typescript 5.6.2

I'm still testing from my local fork but the changes from this PR should be the only ones there.

@jowens
Copy link

jowens commented Nov 19, 2024

OK! Good chance I'm being bit by https://issues.chromium.org/issues/372698905 then.

DELIGHTED that query sets work in Deno here!

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.

4 participants