threads: add component model canonical functions #1783
Conversation
| // Like other component model functions, this is initially added as | ||
| // unshared, meaning spawned threads cannot spawn more threads. In the | ||
| // future this and other component model intrinsics (e.g., `rep.*`) | ||
| // should be marked `shared` to allow access from a shared context. |
There was a problem hiding this comment.
I'm a bit surprised by this in that I would naively expect that it's ok to flag something as shared. It should be safe I think to import a non-shared function when the actual provided value is a shared function?
There was a problem hiding this comment.
Safe, yes... I think what I was thinking here is that eventually all of these component model canonical functions need to be accessible from both shared and unshared contexts. So eventually they should all be marked shared if this proposal is enabled, I guess. But that seems a bit disruptive at this point... I'm just keeping the status quo at this point (inconsistently, though, because I marked thread.hw_concurrency as shared...)
There was a problem hiding this comment.
Hm actually let me phrase what I'm saying a different way. Right now there's no subtyping relationship (at least not in wasm-tools) between shared types and unshared types. That means that you can't import a non-shared function and then satisfy that import with a shared function.
The comment here seems to indicate that you want to work with these intrinsics as-is but you're unable to because the current typing rules require that the core module imports a shared version of the component intrinsic. This version of the intrinsic cannot be imported since core tooling can't emit shared imports yet.
I'd personally like to propose the question to the shared-everything-threads proposal: should shared things be a subtype of non-shared things? Doing so would solve your concern here because the intrinsic would be marked shared and if it were imported as non-shared then that would also work ok because of subtyping. To me at least this feels similar to nullable subtyping.
There was a problem hiding this comment.
What I meant by the comment is that I'm trying to avoid binary changes that users might not expect just yet. I don't think we need sub-typing at the moment, though it might be a worthwhile discussion to have (or revisit: I think we've had it before and concluded that there will be no sub-typing between sharedness; the closest I can find is "shared and unshared types are never subtypes of one another" in this section).
I don't think we'll be able to work with these intrinsics as-is: we will have to mark them shared to be available to multiple threads, it's just that I didn't feel comfortable doing that just yet. But it does seem like what I sketched out above is what needs to happen eventually: if shared-everything-threads is enabled, all CM intrinsics get marked shared. Otherwise they are inaccessible from threads!
There was a problem hiding this comment.
Ah ok. I think though that this should be marked as shared since that's sort of its purpose for existence? I don't think it's going to harm anything because it's all gated for now. Are you worried about it not being usable by core modules today though?
The shared-everything-threads [proposal] adds two new component model canonical functions, `thread.spawn` and `thread.hw_concurrency`. This change adds initial support for these new functions, which should match what is specified in the [canonical ABI]. All of this support should be gated behind checks that ensure the shared-everything-threads proposal has been enabled. [proposal]: https://github.com/WebAssembly/shared-everything-threads [canonical ABI]: https://github.com/WebAssembly/component-model/blob/main/design/mvp/CanonicalABI.md#-canon-threadspawn
abrown
force-pushed
the
set-builtins
branch
from
99e76d9 to
f62b585
Compare
The shared-everything-threads proposal adds two new component model canonical functions,
thread.spawnandthread.hw_concurrency. This change adds initial support for these new functions, which should match what is specified in the canonical ABI. All of this support should be gated behind checks that ensure the shared-everything-threads proposal has been enabled.