Implement component model resources in Wasmtime#6691
Merged
alexcrichton merged 49 commits intobytecodealliance:mainfrom Jul 22, 2023
Merged
Implement component model resources in Wasmtime#6691alexcrichton merged 49 commits intobytecodealliance:mainfrom
alexcrichton merged 49 commits intobytecodealliance:mainfrom
Conversation
Subscribe to Label Actioncc @peterhuene DetailsThis issue or pull request has been labeled: "cranelift", "cranelift:wasm", "wasmtime:api"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
sunfishcode
reviewed
Jul 5, 2023
This commit fixes a minor issue in `FunctionIndices::link_and_append_code` which previously ended up only filling out the `wasm_to_native_trampolines` field for the first module rather than all the modules. Additionally the first module might have too many entries that encompass all modules instead of just its own entries. The fix in this commit is to refactor this logic to ensure that the necessary maps are present for all translations. While technically a bug that can be surfaced through the embedder API it's pretty obscure. The given test here panics beforehand but succeeds afterwards, but this is moreso prep for some future resource-related work where this map will need persisting into the component metadata side of things.
Lots of bits and pieces squashed into this commit. Much to be done still.
Also add a test which requires host-defined drop to be called which isn't working.
No need to check for a null funcref when we already know ahead of time if it's ever going to be null or not.
Plumb around dynamic information about resource types.
Implemented both in the raw wasm intrinsic as well as the host.
Member
Author
|
Ok I think this amounts for everything except |
sunfishcode
reviewed
Jul 6, 2023
The MAY_ENTER flag must always be checked, regardless of whether there's an actual destructor or not.
Member
Author
|
Thanks again for reviewing this @fitzgen, I know it was a big ask! I'm gonna go ahead and queue this up for merging, and leave the newtype index for libcalls as a follow-up if that's ok, but otherwise I think I've addressed the other feedback you had |
They all involve compilation which takes too long and doesn't currently work
geekbeast
pushed a commit
to geekbeast/wasmtime
that referenced
this pull request
Aug 1, 2023
* main: (47 commits) Add core dump support to the runtime (bytecodealliance#6513) Resource table tracks child relationships (bytecodealliance#6779) Wasmtime: Move `OnDemandInstanceAllocator` to its own module (bytecodealliance#6790) wasi: Test the stdio streams implementation (bytecodealliance#6764) Don't generate same-named imports in fact modules (bytecodealliance#6783) Wasmtime: Add support for Wasm tail calls (bytecodealliance#6774) Cranelift: Fix `ABIMachineSpec::gen_add_imm` for riscv64 (bytecodealliance#6780) Update the wasm-tools family of crates, disallow empty component types (bytecodealliance#6777) Fix broken link to WASI API documentation (bytecodealliance#6775) A bunch of cleanups for cranelift-codegen-meta (bytecodealliance#6772) Implement component-to-component calls with resources (bytecodealliance#6769) Ignore async_stack_size if async_support is disabled (bytecodealliance#6771) A bunch of minor cleanups (bytecodealliance#6767) Fix flaky tests in preview2 streams (bytecodealliance#6763) Refactor and simplify component trampolines (bytecodealliance#6751) Cranelift: Implement tail calls on riscv64 (bytecodealliance#6749) WASI Preview 2: rewrite streams and pollable implementation (bytecodealliance#6556) cranelift-wasm: Add support for translating Wasm tail calls to CLIF (bytecodealliance#6760) Cranelift: Get tail calls working on aarch64 (bytecodealliance#6723) Implement component model resources in Wasmtime (bytecodealliance#6691) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is an implementation of the resource datatype in the component model as specified upstream. The goal of this PR is to get lots of the low-level infrastructure for resources sorted out, but not 100% of the story as there's still remaining work. Features implemented in this PR are:
resource.{rep,new,drop}This is intended to be a solid implementation for all the "internals" of resources throughout Wasmtime. It's expected that all further work will be much easier, less invasive, and not so large a scale. Or at least that's the hope. Note though that this is not a 100% complete story for resources in Wasmtime. For example it's still not possible to take a WIT off the shelf with resources and use that with Wasmtime. Key missing features in Wasmtime related to resources are:
bindgen!macro #6722This work is a prerequisite for the above, though, and the hope additionally is that the above can all start to progress in parallel with this work as a basis. Basically getting enough in that everything is no longer bottlenecked on me but it's possible to start building out from here.
Closes #6583