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

Implementing mapping in multithreaded WASM #248

Closed
kainino0x opened this issue Nov 22, 2023 · 3 comments
Closed

Implementing mapping in multithreaded WASM #248

kainino0x opened this issue Nov 22, 2023 · 3 comments
Labels
threading Thread safety and WASM-JS threading problems wasm Issues with WebAssembly targets

Comments

@kainino0x
Copy link
Collaborator

I've given this problem a lot of thought but I can't find a filed issue about it, so here's a quick note.

In native, threads freely do things with whatever objects they want (gpuweb/gpuweb#476) and that includes mapping and unmapping buffers.

However in JS, a buffer can only be unmapped from the thread where it's mapped, and ArrayBuffers for the mapping only exist on that thread. However since there is currently no way to map that ArrayBuffer into the WASM heap, Emscripten just mallocs WASM heap memory for the map, and copies to/from it as needed.

Possible ways to resolve this:

  • Require Get(Const)MappedRange and Unmap to be called from the mapping thread even on native. Problematic for green threads (see [Push|Pop]ErrorScope thread local error stacks #211).
  • In WASM, always forward map and unmap operations to some other helper thread (maybe the main thread). This works as long as we're emulating mapping, and most likely when we gain some kind of mmap-like ability there will be some way to make it visible to all threads. However this does have some overhead, and in some cases could require a dedicated extra thread. Maybe the application can somehow tell us what thread should be the helper thread.
@kainino0x kainino0x added threading Thread safety and WASM-JS threading problems wasm Issues with WebAssembly targets labels Nov 22, 2023
@kainino0x
Copy link
Collaborator Author

kainino0x commented Nov 22, 2023

Also possibly relevant: spontaneous-mode MapAsync (in the Futures version of the API, #199) will complete mappings on arbitrary threads in native. For example, it could happen inside an innocuous Submit from an application thread. Unless we check which thread we're running on and prevent this from happening.

@kainino0x kainino0x added !discuss Needs discussion (at meeting or online) needs docs Non-trivial API contract needs to be documented. Orthogonal to open vs closed; remove when doc'd. labels Feb 28, 2024
@kainino0x kainino0x removed the !discuss Needs discussion (at meeting or online) label Mar 28, 2024
@kainino0x
Copy link
Collaborator Author

Mar 28 meeting:

  • (explanation of problem)
  • Try to enforce this in native?
    • e.g. debug assert if you do things on wrong threads?
  • CF: wgpu recently removed Send+Sync in Wasm
  • CF: Wasm bindings could have a nice error to tell you what happened
  • Wasm bindings likely to be able to paper over this in the long term (see discussion)
  • CF: Users don’t want to be restricted. In native there are lots of thread pools and things that make this impractical.
  • Probably don’t try to restrict this in native.
  • (discussion of how multithreading will work)
    • Before JS gets multithreading, Emscripten may have a mode to proxy all WebGPU calls to one thread
      • wgpu on wasm probably won’t do this
    • After JS gets multithreading, Emscripten could proxy just the map/unmap calls (and it could do so by default because those calls aren’t hot)
    • JS multithreading probably won’t happen until we have a solution to the thread-sharing problem (using objects across threads without postMessage) - expecting to have some kind of thread-shared table to store WebGPU objects in, instead of using a separate table on each thread
      • Encoders are expected to remain single-threaded because they’re hot and don’t really need to be sharable

@kainino0x
Copy link
Collaborator Author

Closing (but with "needs docs")

@kainino0x kainino0x removed the needs docs Non-trivial API contract needs to be documented. Orthogonal to open vs closed; remove when doc'd. label Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
threading Thread safety and WASM-JS threading problems wasm Issues with WebAssembly targets
Projects
None yet
Development

No branches or pull requests

1 participant