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(op_crates): webgpu #7977

Merged
merged 184 commits into from
Mar 1, 2021
Merged

feat(op_crates): webgpu #7977

merged 184 commits into from
Mar 1, 2021

Conversation

crowlKats
Copy link
Member

@crowlKats crowlKats commented Oct 14, 2020

Closes #7863

missing:

  • create navigator & workerNavigator and add webgpu to them
  • create a webgpu instance with the op_webgpu_create_instance op now handled at init and set in OpState
  • get op_webgpu_buffer_get_map_async to work
  • have the slice in op_webgpu_buffer_get_mapped_range be put into zero_copy
  • fix PassChannel structs
  • TS types
  • Move TS types to unstable (do this in follow up)
  • WPT tests (do this in follow up)
  • Resource cleanup
  • WebIDL
  • Implement Deno.customInspect
  • add [[device]] internal slot to all objects except adapter
  • closes resources on finalization for consumables
  • Cleanup automatically using FinalizationRegistry (do this in follow up)
  • validate that objects belong to same device
  • check if objects are valid before using them (check if rid is present)

Examples can be found at https://github.com/crowlKats/webgpu-examples/

@ry
Copy link
Member

ry commented Feb 25, 2021

I did some size comparisons with main branch:

webgpu
target/debug/deno 248M
target/release/deno 72M

main
target/debug/deno 212M
target/release/deno 66M

Quite a bit larger.

@ry
Copy link
Member

ry commented Feb 26, 2021

This is an epic feature that opens the door to doing serious scientific computing with JavaScript. We will likely be able to run TensorFlow.js on Deno without too much work after this. Having a dynamic runtime with easy GPU access is a game changer.

@crowlKats and @lucacasonato have been working on this for months - it's hard to understate the effort that they have put in to deliver this branch. Thank you for your work on this!

  • Superficially the code looks good to me.
  • My main concern is the complete lack of tests. I understand that in the future we are aiming for WPT coverage for WebGpu, but that it will require more work to get there. That's ok, but for the time being we ought to add some basic smoke tests that exercise various parts of the API.
  • I also think a large crate like this should have a README with an introduction to the code base - some description of how it's organized, links to docs, and a todo list future work.
  • It should be noted (probably in README) that CI cannot test because Github Actions runners do not have GPUs (except for windows). This is scary.
  • I am also concerned about the 8mb size increase (this on top of the ICU size increase would make Deno 1.8 50% larger than 1.7) but it doesn't seem like there's much we can do about that.

@crowlKats crowlKats marked this pull request as ready for review February 26, 2021 03:50
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM :)

@crowlKats crowlKats changed the title feat(cli): webgpu feat(op_crates): webgpu Mar 1, 2021
@lucacasonato
Copy link
Member

Huge thanks @crowlKats and @kvark!

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.

WebGPU Implementation
4 participants