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

feat: bring back WebGPU #20812

Merged
merged 48 commits into from Dec 9, 2023
Merged

feat: bring back WebGPU #20812

merged 48 commits into from Dec 9, 2023

Conversation

crowlKats
Copy link
Member

@crowlKats crowlKats commented Oct 6, 2023

Fixes #21514

runtime/build.rs Outdated Show resolved Hide resolved
@sigmaSd
Copy link
Contributor

sigmaSd commented Oct 6, 2023

does this not impact startup time anymore ?

@birkskyum birkskyum mentioned this pull request Oct 9, 2023
@lino-levan
Copy link
Contributor

Linking this PR for context: servo/core-foundation-rs#608

@hastebrot
Copy link

hastebrot commented Oct 11, 2023

if it impacts startup time then it should be put behind a flag, if this helps. otherwise everything will slowdown including deployctl (deno deploy cli) which currently is 20 times faster than az (azure cli tool) just to output the help text on my machine.

.cargo/config.toml Outdated Show resolved Hide resolved
@crowlKats
Copy link
Member Author

We are working on reducing the startup time. A flag would not help. Also, the startup time originally was increased by 3ms, and now is at 1ms increase, and we are working on further reducing it.

@chances
Copy link

chances commented Oct 20, 2023

Requires a wgpu release...

@crowlKats What is missing from wgpu, exactly?

@crowlKats
Copy link
Member Author

Requires a wgpu release...

@crowlKats What is missing from wgpu, exactly?

@chances due to our setup with the wgpu repo, we require the js & rust code to be pulled from a commit that is a release of wgpu.
The blockers for wgpu release from my end is a core-graphics-types release and then a metal-rs release.

Copy link
Contributor

@petamoriken petamoriken left a comment

Choose a reason for hiding this comment

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

Added many suggestions related to inspect 🙇

ext/webgpu/01_webgpu.js Show resolved Hide resolved
ext/webgpu/01_webgpu.js Outdated Show resolved Hide resolved
ext/webgpu/01_webgpu.js Outdated Show resolved Hide resolved
ext/webgpu/01_webgpu.js Outdated Show resolved Hide resolved
ext/webgpu/01_webgpu.js Outdated Show resolved Hide resolved
ext/webgpu/01_webgpu.js Outdated Show resolved Hide resolved
ext/webgpu/01_webgpu.js Outdated Show resolved Hide resolved
ext/webgpu/01_webgpu.js Outdated Show resolved Hide resolved
ext/webgpu/01_webgpu.js Outdated Show resolved Hide resolved
ext/webgpu/02_surface.js Show resolved Hide resolved
crowlKats and others added 11 commits November 29, 2023 09:52
Co-authored-by: Kenta Moriuchi <moriken@kimamass.com>
Signed-off-by: Leo Kettmeir <crowlkats@toaxl.com>
Co-authored-by: Kenta Moriuchi <moriken@kimamass.com>
Signed-off-by: Leo Kettmeir <crowlkats@toaxl.com>
Co-authored-by: Kenta Moriuchi <moriken@kimamass.com>
Signed-off-by: Leo Kettmeir <crowlkats@toaxl.com>
Co-authored-by: Kenta Moriuchi <moriken@kimamass.com>
Signed-off-by: Leo Kettmeir <crowlkats@toaxl.com>
Co-authored-by: Kenta Moriuchi <moriken@kimamass.com>
Signed-off-by: Leo Kettmeir <crowlkats@toaxl.com>
Co-authored-by: Kenta Moriuchi <moriken@kimamass.com>
Signed-off-by: Leo Kettmeir <crowlkats@toaxl.com>
Co-authored-by: Kenta Moriuchi <moriken@kimamass.com>
Signed-off-by: Leo Kettmeir <crowlkats@toaxl.com>
Co-authored-by: Kenta Moriuchi <moriken@kimamass.com>
Signed-off-by: Leo Kettmeir <crowlkats@toaxl.com>
Co-authored-by: Kenta Moriuchi <moriken@kimamass.com>
Signed-off-by: Leo Kettmeir <crowlkats@toaxl.com>
# Conflicts:
#	Cargo.lock
#	Cargo.toml
#	cli/js/40_testing.js
#	tools/README.md
@KnorpelSenf
Copy link
Contributor

This fixes #21514, feel free to update the description

@ry
Copy link
Member

ry commented Dec 8, 2023

@crowlKats can you provide start up time benchmarks? Also max RSS for a blank file?

# Conflicts:
#	Cargo.lock
#	Cargo.toml
@crowlKats
Copy link
Member Author

@ry
Screenshot 2023-12-08 at 16 41 42

@crowlKats
Copy link
Member Author

Memory:
Screenshot 2023-12-08 at 16 59 10
Screenshot 2023-12-08 at 16 59 47

@bartlomieju
Copy link
Member

On my machine:

hyperfine --warmup 30 -S none './target/release/deno run empty.js' 'deno run empty.js'
Benchmark 1: ./target/release/deno run empty.js
  Time (mean ± σ):      16.3 ms ±   1.0 ms    [User: 10.3 ms, System: 3.9 ms]
  Range (min … max):    15.4 ms …  22.5 ms    189 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet PC without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Benchmark 2: deno run empty.js
  Time (mean ± σ):      14.3 ms ±   0.3 ms    [User: 10.1 ms, System: 4.2 ms]
  Range (min … max):    13.8 ms …  16.4 ms    205 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet PC without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Summary
  'deno run empty.js' ran
    1.14 ± 0.07 times faster than './target/release/deno run empty.js'

/usr/bin/time -l ./target/release/deno run empty.js
        0.03 real         0.01 user         0.00 sys
            38584320  maximum resident set size
/usr/bin/time -l deno run empty.js
        0.02 real         0.01 user         0.00 sys
            38273024  maximum resident set size

@mmastrac
Copy link
Member

mmastrac commented Dec 8, 2023

/bench startup_hello_world,startup_nop_empty,startup_nop_trivial

@denobot
Copy link
Collaborator

denobot commented Dec 8, 2023

startup_hello_world

time user system relative
node 27.887 ms 21.973 ms 7.132 ms +194.9%
bun 9.456 ms 4.116 ms 6.413 ms best
deno 15.391 ms 9.995 ms 6.672 ms +62.8%
deno-baseline 15.152 ms 10.052 ms 6.365 ms +60.2%

startup_nop_empty

time user system relative
node 22.526 ms 16.879 ms 6.872 ms +681.6%
bun 2.882 ms 0.918 ms 1.843 ms best
deno 15.263 ms 10.067 ms 6.464 ms +429.6%
deno-baseline 14.955 ms 9.424 ms 6.802 ms +418.9%

startup_nop_trivial

time user system relative
node 21.986 ms 16.613 ms 6.58 ms +92.4%
bun 11.426 ms 5.124 ms 7.47 ms best
deno 15.137 ms 10.211 ms 6.192 ms +32.5%
deno-baseline 14.952 ms 9.681 ms 6.539 ms +30.9%

start: Fri, 08 Dec 2023 16:29:41 GMT

id: 1cab344b-ca4a-4422-a124-4a40b7f564e8

server: 093a6a04-1f84-4244-a37b-9a2e40cc5782

@mmastrac
Copy link
Member

mmastrac commented Dec 8, 2023

Comparing to deno 1.38.5 on my machine after a clean && build I see no impact:

09:52 $ hyperfine --warmup 30 -S none './target/release/deno run /tmp/empty.js' './target/release/deno-2 run /tmp/empty.js'
Benchmark 1: ./target/release/deno run /tmp/empty.js
  Time (mean ± σ):      13.3 ms ±   0.5 ms    [User: 9.9 ms, System: 3.4 ms]
  Range (min … max):    12.6 ms …  16.3 ms    212 runs
 
Benchmark 2: ./target/release/deno-2 run /tmp/empty.js
  Time (mean ± σ):      13.2 ms ±   0.4 ms    [User: 9.8 ms, System: 3.4 ms]
  Range (min … max):    12.4 ms …  15.3 ms    233 runs
 
Summary
  './target/release/deno-2 run /tmp/empty.js' ran
    1.01 ± 0.05 times faster than './target/release/deno run /tmp/empty.js'

cli/js/40_testing.js Show resolved Hide resolved

let isCI: boolean;
try {
isCI = (Deno.env.get("CI")?.length ?? 0) > 0;
Copy link
Member

Choose a reason for hiding this comment

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

This is 4th place we define this env var :) we should move it to test_util.ts in a follow up. Can you handle that @crowlKats?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure

cli/tests/unit/webgpu_test.ts Show resolved Hide resolved
ext/node/polyfill.rs Outdated Show resolved Hide resolved
ext/webgpu/02_surface.js Outdated Show resolved Hide resolved
ext/webgpu/README.md Outdated Show resolved Hide resolved
Comment on lines +266 to +274
gpu: {
configurable: true,
enumerable: true,
get() {
webidl.assertBranded(this, NavigatorPrototype);
loadWebGPU();
return webgpu.gpu;
},
},
Copy link
Member

Choose a reason for hiding this comment

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

What about set()? I think on the web one can override this value

Copy link
Member Author

Choose a reason for hiding this comment

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

nope, not overwritable, however all the navigator getters should have no-op setters. I'll fix that up in a follow up with tests.

@crowlKats crowlKats merged commit 393abed into denoland:main Dec 9, 2023
14 checks passed
@crowlKats crowlKats deleted the webgpu branch December 9, 2023 00:19
@lino-levan
Copy link
Contributor

Nice work Leo (and the whole Deno team). Excited to see this brought back into Deno.

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.

Reintroduce WebGPU API