-
Notifications
You must be signed in to change notification settings - Fork 252
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
Add support for async ABI, futures, streams, and errors #1895
Conversation
Sorry about all the churn in |
Thanks again for this! My current plan is to start tackling this the week-after-next (after WasmCon) but if you've got any high-level points you want me to confirm ahead of time feel free to let me know |
No hurry at all; I'm mainly opening this (and soon-to-follow PRs to |
This adds support for loading, compiling, linking, and running components which use the [Async ABI](https://github.com/WebAssembly/component-model/blob/main/design/mvp/Async.md) along with the [`stream`, `future`, and `error-context`](WebAssembly/component-model#405) types. It also adds support for generating host bindings such that multiple host functions can be run concurrently with guest tasks -- without monopolizing the `Store`. See the [implementation RFC](bytecodealliance/rfcs#38) for details, as well as [this repo](https://github.com/dicej/component-async-demo) containing end-to-end smoke tests. This is very much a work-in progress, with a number of tasks remaining: - [ ] Avoid exposing global task IDs to guests and use per-instance IDs instead - [ ] Track `task.return` type during compilation and assert the actual and expected types match at runtime - [ ] Ensure all guest pointers are bounds-checked when lifting, lowering, or copying values - [ ] Reduce code duplication in `wasmtime_cranelift::compiler::component` - [ ] Add support for `(Typed)Func::call_concurrent` per the RFC - [ ] Add support for multiplexing stream/future reads/writes and concurrent calls to guest exports per the RFC - [ ] Refactor, clean up, and unify handling of backpressure, yields, and even polling - [ ] Guard against reentrance where required (e.g. in certain fused adapter calls) - [ ] Add integration test cases covering new functionality to tests/all/component_model (starting by porting over the tests in https://github.com/dicej/component-async-demo) - [ ] Add binding generation test cases to crates/component-macro/tests - [ ] Add WAST tests to tests/misc_testsuite/component-model - [ ] Add support and test coverage for callback-less async functions (e.g. goroutines) - [ ] Switch to back to upstream `wasm-tools` once bytecodealliance/wasm-tools#1895 has been merged and released Signed-off-by: Joel Dice <joel.dice@fermyon.com>
This adds support for loading, compiling, linking, and running components which use the [Async ABI](https://github.com/WebAssembly/component-model/blob/main/design/mvp/Async.md) along with the [`stream`, `future`, and `error-context`](WebAssembly/component-model#405) types. It also adds support for generating host bindings such that multiple host functions can be run concurrently with guest tasks -- without monopolizing the `Store`. See the [implementation RFC](bytecodealliance/rfcs#38) for details, as well as [this repo](https://github.com/dicej/component-async-demo) containing end-to-end smoke tests. This is very much a work-in progress, with a number of tasks remaining: - [ ] Avoid exposing global task IDs to guests and use per-instance IDs instead - [ ] Track `task.return` type during compilation and assert the actual and expected types match at runtime - [ ] Ensure all guest pointers are bounds-checked when lifting, lowering, or copying values - [ ] Reduce code duplication in `wasmtime_cranelift::compiler::component` - [ ] Reduce code duplication between `StoreContextMut::on_fiber` and `concurrent::on_fiber` - [ ] Minimize and/or document the use of unsafe code - [ ] Add support for `(Typed)Func::call_concurrent` per the RFC - [ ] Add support for multiplexing stream/future reads/writes and concurrent calls to guest exports per the RFC - [ ] Refactor, clean up, and unify handling of backpressure, yields, and even polling - [ ] Guard against reentrance where required (e.g. in certain fused adapter calls) - [ ] Add integration test cases covering new functionality to tests/all/component_model (starting by porting over the tests in https://github.com/dicej/component-async-demo) - [ ] Add binding generation test cases to crates/component-macro/tests - [ ] Add WAST tests to tests/misc_testsuite/component-model - [ ] Add support and test coverage for callback-less async functions (e.g. goroutines) - [ ] Switch to back to upstream `wasm-tools` once bytecodealliance/wasm-tools#1895 has been merged and released Signed-off-by: Joel Dice <joel.dice@fermyon.com>
This adds support for loading, compiling, linking, and running components which use the [Async ABI](https://github.com/WebAssembly/component-model/blob/main/design/mvp/Async.md) along with the [`stream`, `future`, and `error-context`](WebAssembly/component-model#405) types. It also adds support for generating host bindings such that multiple host functions can be run concurrently with guest tasks -- without monopolizing the `Store`. See the [implementation RFC](bytecodealliance/rfcs#38) for details, as well as [this repo](https://github.com/dicej/component-async-demo) containing end-to-end smoke tests. This is very much a work-in progress, with a number of tasks remaining: - [ ] Avoid exposing global task IDs to guests and use per-instance IDs instead - [ ] Track `task.return` type during compilation and assert the actual and expected types match at runtime - [ ] Ensure all guest pointers are bounds-checked when lifting, lowering, or copying values - [ ] Reduce code duplication in `wasmtime_cranelift::compiler::component` - [ ] Reduce code duplication between `StoreContextMut::on_fiber` and `concurrent::on_fiber` - [ ] Minimize and/or document the use of unsafe code - [ ] Add support for `(Typed)Func::call_concurrent` per the RFC - [ ] Add support for multiplexing stream/future reads/writes and concurrent calls to guest exports per the RFC - [ ] Refactor, clean up, and unify handling of backpressure, yields, and even polling - [ ] Guard against reentrance where required (e.g. in certain fused adapter calls) - [ ] Add integration test cases covering new functionality to tests/all/component_model (starting by porting over the tests in https://github.com/dicej/component-async-demo) - [ ] Add binding generation test cases to crates/component-macro/tests - [ ] Add WAST tests to tests/misc_testsuite/component-model - [ ] Add support and test coverage for callback-less async functions (e.g. goroutines) - [ ] Switch to back to upstream `wasm-tools` once bytecodealliance/wasm-tools#1895 has been merged and released Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix clippy warnings and bench/fuzzing errors Signed-off-by: Joel Dice <joel.dice@fermyon.com> revert atomic.wit whitespace change Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix build when component-model disabled Signed-off-by: Joel Dice <joel.dice@fermyon.com> bless component-macro expected output Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix no-std build error Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix build with --no-default-features --features runtime,component-model Signed-off-by: Joel Dice <joel.dice@fermyon.com> partly fix no-std build It's still broken due to the use of `std::collections::HashMap` in crates/wasmtime/src/runtime/vm/component.rs. I'll address that as part of the work to avoid exposing global task/future/stream/error-context handles to guests. Signed-off-by: Joel Dice <joel.dice@fermyon.com> maintain per-instance tables for futures, streams, and error-contexts Signed-off-by: Joel Dice <joel.dice@fermyon.com>
This adds support for loading, compiling, linking, and running components which use the [Async ABI](https://github.com/WebAssembly/component-model/blob/main/design/mvp/Async.md) along with the [`stream`, `future`, and `error-context`](WebAssembly/component-model#405) types. It also adds support for generating host bindings such that multiple host functions can be run concurrently with guest tasks -- without monopolizing the `Store`. See the [implementation RFC](bytecodealliance/rfcs#38) for details, as well as [this repo](https://github.com/dicej/component-async-demo) containing end-to-end smoke tests. This is very much a work-in progress, with a number of tasks remaining: - [ ] Avoid exposing global task IDs to guests and use per-instance IDs instead - [ ] Track `task.return` type during compilation and assert the actual and expected types match at runtime - [ ] Ensure all guest pointers are bounds-checked when lifting, lowering, or copying values - [ ] Reduce code duplication in `wasmtime_cranelift::compiler::component` - [ ] Reduce code duplication between `StoreContextMut::on_fiber` and `concurrent::on_fiber` - [ ] Minimize and/or document the use of unsafe code - [ ] Add support for `(Typed)Func::call_concurrent` per the RFC - [ ] Add support for multiplexing stream/future reads/writes and concurrent calls to guest exports per the RFC - [ ] Refactor, clean up, and unify handling of backpressure, yields, and even polling - [ ] Guard against reentrance where required (e.g. in certain fused adapter calls) - [ ] Add integration test cases covering new functionality to tests/all/component_model (starting by porting over the tests in https://github.com/dicej/component-async-demo) - [ ] Add binding generation test cases to crates/component-macro/tests - [ ] Add WAST tests to tests/misc_testsuite/component-model - [ ] Add support and test coverage for callback-less async functions (e.g. goroutines) - [ ] Switch to back to upstream `wasm-tools` once bytecodealliance/wasm-tools#1895 has been merged and released Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix clippy warnings and bench/fuzzing errors Signed-off-by: Joel Dice <joel.dice@fermyon.com> revert atomic.wit whitespace change Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix build when component-model disabled Signed-off-by: Joel Dice <joel.dice@fermyon.com> bless component-macro expected output Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix no-std build error Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix build with --no-default-features --features runtime,component-model Signed-off-by: Joel Dice <joel.dice@fermyon.com> partly fix no-std build It's still broken due to the use of `std::collections::HashMap` in crates/wasmtime/src/runtime/vm/component.rs. I'll address that as part of the work to avoid exposing global task/future/stream/error-context handles to guests. Signed-off-by: Joel Dice <joel.dice@fermyon.com> maintain per-instance tables for futures, streams, and error-contexts Signed-off-by: Joel Dice <joel.dice@fermyon.com> refactor task/stream/future handle lifting/lowering This addresses a couple of issues: - Previously, we were passing task/stream/future/error-context reps directly to instances while keeping track of which instance had access to which rep. That worked fine in that there was no way to forge access to inaccessible reps, but it leaked information about what other instances were doing. Now we maintain per-instance waitable and error-context tables which map the reps to and from the handles which the instance sees. - The `no_std` build was broken due to use of `HashMap` in `runtime::vm::component`, which is now fixed. Note that we use one single table per instance for all tasks, streams, and futures. This is partly necessary because, when async events are delivered to the guest, it wouldn't have enough context to know which stream or future we're talking about if each unique stream and future type had its own table. So at minimum, we need to use the same table for all streams (regardless of payload type), and likewise for futures. Also, per WebAssembly/component-model#395 (comment), the plan is to move towards a shared table for all resource types as well, so this moves us in that direction. Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix wave breakage due to new stream/future/error-context types Signed-off-by: Joel Dice <joel.dice@fermyon.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything's looking great here, thanks again for your work here! I've left a number of mostly stylistic things here and there, nothing major. I still haven't gotten to most of wit-component
though which I'll tackle tomorrow.
One thing though I'd like to ask for, if you're up for it here, is to integrate some of this into fuzzing. The main way we fuzz components right now is via the wit-smith
crate and the roundtrip_wit
fuzzer. Could you extend wit-smith
with support for the new types of stream/future/error-context? (I think this should be relatively easy). Afterwards if you run the roundtrip_wit
fuzzer for a bit that should hopefully have no issues.
After that though there's a perhaps slightly more substantial part, which is can you add async support to wasm-tools component embed --dummy
? I'm imagining something like --async
which assumes that all imports/exports are async (or maybe --async-callback
or --async-stackful
as well?). That would then import all the various intrinsics. I ask this because it's useful for two primary reason:
- Mostly it enables more fuzzing. This helps exercise everything in
wit-component
to ensure that core modules that do async things can always be turned into a component. - Otherwise it helps for exploring things at a low level. For example if you're curious to remember what the async core signatures look like you can run
--dummy --async-callbacks
(or w/e the flags are) to see the core wasm module shape that needs to be produced.
This adds support for encoding and parsing components which use the [Async ABI](https://github.com/WebAssembly/component-model/blob/main/design/mvp/Async.md) and associated canonical options and functions, along with the [`stream`, `future`, and `error`](WebAssembly/component-model#405) types. See bytecodealliance/rfcs#38 for more context. Signed-off-by: Joel Dice <joel.dice@fermyon.com> add wasmparser::WasmFeatures support to wasm-compose Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix no-std build in readers.rs Signed-off-by: Joel Dice <joel.dice@fermyon.com> rename `error` to `error-context` per latest spec Signed-off-by: Joel Dice <joel.dice@fermyon.com> rename `error` to `error-context` per latest spec (part 2) Also, parse string encoding and realloc from encoded `error-context.new` and `error-context.debug-string` names. Signed-off-by: Joel Dice <joel.dice@fermyon.com> add `wast` support for parsing async canon opts And add tests/local/component-model-async/lift-async.wast for round-trip testing of async lifts (more to come). Signed-off-by: Joel Dice <joel.dice@fermyon.com> more wast async support and more tests This also fixes a bug in `wasmprinter` keeping track of core functions. Signed-off-by: Joel Dice <joel.dice@fermyon.com> more wast async support; add async tests; fix bugs Signed-off-by: Joel Dice <joel.dice@fermyon.com> more component-model-async tests and fixes Signed-off-by: Joel Dice <joel.dice@fermyon.com> add `wit-parser` tests for streams, futures, and error-contexts Signed-off-by: Joel Dice <joel.dice@fermyon.com> add first `wit-component` async test This required adding a new `wit_parser::decoding::decode_reader_with_features` function for passing `WasmFeatures` to `wasmparser::Validator::new_with_features`. Signed-off-by: Joel Dice <joel.dice@fermyon.com> add more async tests Signed-off-by: Joel Dice <joel.dice@fermyon.com> add `async-builtins` test for `wit-component` Signed-off-by: Joel Dice <joel.dice@fermyon.com> add `async-streams-and-futures` test to `wit-component` Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix stream/future type handling for exported interfaces Signed-off-by: Joel Dice <joel.dice@fermyon.com> support callback-less (AKA stackful) async lifts Signed-off-by: Joel Dice <joel.dice@fermyon.com> address review feedback Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Thanks for the thorough review as always, @alexcrichton . I believe I've addressed all your inline comments with either code changes or explanations. I'll work on the fuzzing and |
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
let Some(SubType { | ||
composite_type: | ||
CompositeType { | ||
inner: CompositeInnerType::Func(_), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit of a spec-level question, but at least personally I'm surprised how lax the validation is here. I would have expected this to be tagged with a component value type and the core function signature is inferred from that. With the current definition it seems like you could use the same task.return
intrinsic for multiple component value types so long as they have the same ABI, and that doesn't feel great to me because we typically have more static information than that in the component model.
Does what I'm saying make sense though? To be clear I'm not saying that this can't work (as it clearly does), just that I'd probably lean towards some more validation and/or a different structure here. Should I take this upstream to the spec itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I've struggled with this myself, and indeed my first draft (written before a spec existed) used a component function type instead of a core function type. After all, you can always derive the latter from the former. Luke and I discussed it and landed on the current scheme, but I don't remember the details as to why. Anyway, yes, please open an issue upstream about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chatted with Luke about this, and he'll update the spec to use the component-level return type instead of the core function type for task.return
declarations, then I'll update the implementation to match.
This fixes `cargo check --no-default-features --feature compose`. Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
I didn't have time today to do the fuzzing and |
Oh I thought I wrote this down last Friday but I appear to have forgotten, sorry about that! In any case if you'd like I think it's reasonable to merge this as-is and do the dummy/fuzzing bits as follow-ups. |
Either way works for me. There are three to-do items at this point:
If you think all those are fine to leave as follow-ups, feel free to merge. Either way, I'll aim to address those items ASAP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I'll go ahead and flag this for merge to prevent any possible future merge conflicts, and sounds good to me 👍
…ance#1895) * Add support for async ABI, futures, streams, and errors This adds support for encoding and parsing components which use the [Async ABI](https://github.com/WebAssembly/component-model/blob/main/design/mvp/Async.md) and associated canonical options and functions, along with the [`stream`, `future`, and `error`](WebAssembly/component-model#405) types. See bytecodealliance/rfcs#38 for more context. Signed-off-by: Joel Dice <joel.dice@fermyon.com> add wasmparser::WasmFeatures support to wasm-compose Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix no-std build in readers.rs Signed-off-by: Joel Dice <joel.dice@fermyon.com> rename `error` to `error-context` per latest spec Signed-off-by: Joel Dice <joel.dice@fermyon.com> rename `error` to `error-context` per latest spec (part 2) Also, parse string encoding and realloc from encoded `error-context.new` and `error-context.debug-string` names. Signed-off-by: Joel Dice <joel.dice@fermyon.com> add `wast` support for parsing async canon opts And add tests/local/component-model-async/lift-async.wast for round-trip testing of async lifts (more to come). Signed-off-by: Joel Dice <joel.dice@fermyon.com> more wast async support and more tests This also fixes a bug in `wasmprinter` keeping track of core functions. Signed-off-by: Joel Dice <joel.dice@fermyon.com> more wast async support; add async tests; fix bugs Signed-off-by: Joel Dice <joel.dice@fermyon.com> more component-model-async tests and fixes Signed-off-by: Joel Dice <joel.dice@fermyon.com> add `wit-parser` tests for streams, futures, and error-contexts Signed-off-by: Joel Dice <joel.dice@fermyon.com> add first `wit-component` async test This required adding a new `wit_parser::decoding::decode_reader_with_features` function for passing `WasmFeatures` to `wasmparser::Validator::new_with_features`. Signed-off-by: Joel Dice <joel.dice@fermyon.com> add more async tests Signed-off-by: Joel Dice <joel.dice@fermyon.com> add `async-builtins` test for `wit-component` Signed-off-by: Joel Dice <joel.dice@fermyon.com> add `async-streams-and-futures` test to `wit-component` Signed-off-by: Joel Dice <joel.dice@fermyon.com> fix stream/future type handling for exported interfaces Signed-off-by: Joel Dice <joel.dice@fermyon.com> support callback-less (AKA stackful) async lifts Signed-off-by: Joel Dice <joel.dice@fermyon.com> address review feedback Signed-off-by: Joel Dice <joel.dice@fermyon.com> * address more review feedback Signed-off-by: Joel Dice <joel.dice@fermyon.com> * fix test build regression Signed-off-by: Joel Dice <joel.dice@fermyon.com> * fix more test regressions Signed-off-by: Joel Dice <joel.dice@fermyon.com> * remove need for `Default` impls in `wast::component::func` Signed-off-by: Joel Dice <joel.dice@fermyon.com> * fix another test regression Signed-off-by: Joel Dice <joel.dice@fermyon.com> * minor code simplification Signed-off-by: Joel Dice <joel.dice@fermyon.com> * require wasmparser `features` feature in wasm-compose This fixes `cargo check --no-default-features --feature compose`. Signed-off-by: Joel Dice <joel.dice@fermyon.com> * require wasmparser `features` feature in wit-parser Signed-off-by: Joel Dice <joel.dice@fermyon.com> * simplify `r#async` parsing code Signed-off-by: Joel Dice <joel.dice@fermyon.com> * add a bunch of comments to wit-component async code Signed-off-by: Joel Dice <joel.dice@fermyon.com> * allow importing the same function sync and async Signed-off-by: Joel Dice <joel.dice@fermyon.com> --------- Signed-off-by: Joel Dice <joel.dice@fermyon.com>
This adds support for encoding and parsing components which use the Async ABI and associated canonical options and functions, along with the
stream
,future
, anderror
types.Note that theEDIT: doneerror
type was recently (about 30 minutes ago) renamed toerror-context
in Luke's spec PR. I haven't updated this implementation to reflect that yet, but will do so in a follow-up commit. That should allow us to avoid conflicts with existing WIT files that useerror
as a type and/or interface name.This does not yet include any new tests; I'll also add those in a follow-up commit.
See bytecodealliance/rfcs#38 for more context.
This is ready for review, but I'll leave it in draft mode until the following are complete:
error
toerror-context
tests/local/component-model
andtests/local/missing-features
to exercise newwasm-encoder
,wasmparser
, andwasmprinter
featurescrates/wit-component/tests
to exercise its new featurescrates/wit-parser/tests
to exercise its new features