Open alexcrichton opened 11 months ago
My opinion is that, since wasi-threads is not compatible with the component model and is currently being set aside in order to investigate core wasm primitives, we should not port wasi-threads to the new wasmtime-wasi implementation, and unship support for wasi-threads when we retire wasi-common.
I think we should keep wasi-threads around at least until the next thing is available: it would be great to be able to compare and contrast, e.g., for measuring spawn performance or for troubleshooting bugs. I am hoping to work on whatever is next soon but am waiting for some things to be sorted out on the spec side of things; so it's not clear to me when that switch over will be.
Some thoughts:
wasi-threads
? I'm not sure I completely understand why removing wasi-common
requires removing wasi-threads
. Is it just the dependency? Can't wasi-threads still work for non-component modules?&mut self
to &self
issues at some point with the next thing? The next thing will still need to access these futures from multiple threads, no? Maybe I just need to dig into this more to understand better, but I'm wondering out loud if this problem won't go away so we should just deal with it now.do we have to remove wasi-threads? I'm not sure I completely understand why removing wasi-common requires removing wasi-threads. Is it just the dependency? Can't wasi-threads still work for non-component modules?
wasmtime-wasi::preview2
contains a complete replacement for wasi-common
in every regard except for supporting wasi-threads, so we'd like to retire wasi-common
and remove it from the tree.
aren't we going to have to deal with &mut self to &self issues at some point with the next thing? The next thing will still need to access these futures from multiple threads, no? Maybe I just need to dig into this more to understand better, but I'm wondering out loud if this problem won't go away so we should just deal with it now.
I don't want to make architectural decisions about wasmtime-wasi::preview2
based on an expectation of how some engineering work in the future might work out, but instead based on how it does work out, because that architectural change could be very costly in terms of both engineering time, complexity of this new implementation, and performance of the single-threaded use case. I do not believe we have enough information now to make this choice.
As part of the effort to implement the upcoming 0.2.0 version of WASI the previous implementation of WASI in Wasmtime, the
wasi-common
crate, was rewritten and is now located in thewasmtime-wasi
crate in thepreview2
submodule. This means that Wasmtime (and thewasmtime
CLI) now has two entirely separate implementations of the preview1 snapshot of WASI. This is not something I think we should retain indefinitely, so I think it's worthwhile to plan for the removal of the oldwasi-common
crate.The only technical blocker I'm aware of for this is that the preview2-based implementation of
wasi_snapshot_preview1
does not support WASI threads. Effectively if a thread is spawned then all WASI functions will stop working because the WASI context is no longer uniquely owned. More-or-less this boils down to this panic which would be dynamically triggered. There is no way right now to share the WASI context amongst threads and have functions continue to work (e.g.poll
working concurrently on multiple threads).There's definitely a lot of issues on the surface that look like incompatibilities, such as liberal usage of
&mut Table
and&mut self
, but I think much of this can be updated with relative ease to work with&Table
that has some internal synchronization which gives temporary access to&mut self
on various objects. The main thing I don't know how to solve at this time, however, is how blocking works.Currently all blocking computations in Wasmtime's implementation of WASI are represented as a
Future
. This is chiefly done through theSubscribe
trait:The usage of
&mut self
here is problematic because the same I/O object could be subscribe to from multiple threads which means&mut
exclusive access is not possible. More-or-less what theoretically needs to happen is to push the&self
into this method to enable waiting for readiness on objects on multiple threads simultaenously. Each thread would create its own future inpoll
and then it'd get arbitrated internally. This is, however, a very large refactoring and departure from the current design, one that I don't think can easily be done and I think may have unknown consequences (e.g. more-than-expected overhead in the single-threaded use case).This conclusion brings me to opening this issue for more discussion.
cc @abrown