-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Redesign interface type value representation #4198
Merged
alexcrichton
merged 1 commit into
bytecodealliance:main
from
alexcrichton:no-component-value
Jun 1, 2022
Merged
Redesign interface type value representation #4198
alexcrichton
merged 1 commit into
bytecodealliance:main
from
alexcrichton:no-component-value
Jun 1, 2022
Conversation
This file contains 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
github-actions
bot
added
the
wasmtime:api
Related to the API of the `wasmtime` crate itself
label
May 31, 2022
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasmtime:api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
42 tasks
Prior to this PR a major feature of calling component exports (bytecodealliance#4039) was the usage of the `Value<T>` type. This type represents a value stored in wasm linear memory (the type `T` stored there). This implementation had a number of drawbacks though: * When returning a value it's ABI-specific whether you use `T` or `Value<T>` as a return value. If `T` is represented with one wasm primitive then you have to return `T`, otherwise the return value must be `Value<T>`. This is somewhat non-obvious and leaks ABI-details into the API which is unfortunate. * The `T` in `Value<T>` was somewhat non-obvious. For example a wasm-owned string was `Value<String>`. Using `Value<&str>` didn't work. * Working with `Value<T>` was unergonomic in the sense that you had to first "pair" it with a `&Store<U>` to get a `Cursor<T>` and then you could start reading the value. * Custom structs and enums, while not implemented yet, were planned to be quite wonky where when you had `Cursor<MyStruct>` then you would have to import a `CursorMyStructExt` trait generated by a proc-macro (think a `#[derive]` on the definition of `MyStruct`) which would enable field accessors, returning cursors of all the fields. * In general there was no "generic way" to load a `T` from memory. Other operations like lift/lower/store all had methods in the `ComponentValue` trait but load had no equivalent. None of these drawbacks were deal-breakers per-se. When I started to implement imported functions, though, the `Value<T>` type no longer worked. The major difference between imports and exports is that when receiving values from wasm an export returns at most one wasm primitive where an import can yield (through arguments) up to 16 wasm primitives. This means that if an export returned a string it would always be `Value<String>` but if an import took a string as an argument there was actually no way to represent this with `Value<String>` since the value wasn't actually stored in memory but rather the pointer/length pair is received as arguments. Overall this meant that `Value<T>` couldn't be used for arguments-to-imports, which means that altogether something new would be required. This PR completely removes the `Value<T>` and `Cursor<T>` type in favor of a different implementation. The inspiration from this comes from the fact that all primitives can be both lifted and lowered into wasm while it's just some times which can only go one direction. For example `String` can be lowered into wasm but can't be lifted from wasm. Instead some sort of "view" into wasm needs to be created during lifting. One of the realizations from bytecodealliance#4039 was that we could leverage run-time-type-checking to reject static constructions that don't make sense. For example if an embedder asserts that a wasm function returns a Rust `String` we can reject that at typechecking time because it's impossible for a wasm module to ever do that. The new system of imports/exports in this PR now looks like: * Type-checking takes into accont an `Op` operation which indicates whether we'll be lifting or lowering the type. This means that we can allow the lowering operation for `String` but disallow the lifting operation. While we can't statically rule out an embedder saying that a component returns a `String` we can now reject it at runtime and disallow it from being called. * The `ComponentValue` trait now sports a new `load` function. This function will load and instance of `Self` from the byte-array provided. This is implemented for all types but only ever actually executed when the `lift` operation is allowed during type-checking. * The `Lift` associated type is removed since it's now expected that the lift operation returns `Self`. * The `ComponentReturn` trait is now no longer necessary and is removed. Instead returns are bounded by `ComponentValue`. During type-checking it's required that the return value can be lifted, disallowing, for example, returning a `String` or `&str`. * With `Value` gone there's no need to specify the ABI details of the return value, or whether it's communicated through memory or not. This means that handling return values through memory is transparently handled by Wasmtime. * Validation is in a sense more eagerly performed now. Whenever a value `T` is loaded the entire immediate structure of `T` is loaded and validated. Note that recursive through memory validation still does not happen, so the contents of lists or strings aren't validated, it's just validated that the pointers are in-bounds. Overall this felt like a much clearer system to work with and should be much easier to integrate with imported functions as well. The new `WasmStr` and `WasmList<T>` types can be used in import arguments and lifted from the immediate arguments provided rather than forcing them to always be stored in memory.
alexcrichton
force-pushed
the
no-component-value
branch
from
June 1, 2022 18:07
42df352
to
49bb3ea
Compare
fitzgen
approved these changes
Jun 1, 2022
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.
Nice!
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.
Prior to this PR a major feature of calling component exports (#4039)
was the usage of the
Value<T>
type. This type represents a valuestored in wasm linear memory (the type
T
stored there). Thisimplementation had a number of drawbacks though:
When returning a value it's ABI-specific whether you use
T
orValue<T>
as a return value. IfT
is represented with one wasmprimitive then you have to return
T
, otherwise the return value mustbe
Value<T>
. This is somewhat non-obvious and leaks ABI-details intothe API which is unfortunate.
The
T
inValue<T>
was somewhat non-obvious. For example awasm-owned string was
Value<String>
. UsingValue<&str>
didn'twork.
Working with
Value<T>
was unergonomic in the sense that you had tofirst "pair" it with a
&Store<U>
to get aCursor<T>
and then youcould start reading the value.
Custom structs and enums, while not implemented yet, were planned to
be quite wonky where when you had
Cursor<MyStruct>
then you wouldhave to import a
CursorMyStructExt
trait generated by a proc-macro(think a
#[derive]
on the definition ofMyStruct
) which wouldenable field accessors, returning cursors of all the fields.
In general there was no "generic way" to load a
T
from memory. Otheroperations like lift/lower/store all had methods in the
ComponentValue
trait but load had no equivalent.None of these drawbacks were deal-breakers per-se. When I started
to implement imported functions, though, the
Value<T>
type no longerworked. The major difference between imports and exports is that when
receiving values from wasm an export returns at most one wasm primitive
where an import can yield (through arguments) up to 16 wasm primitives.
This means that if an export returned a string it would always be
Value<String>
but if an import took a string as an argument there wasactually no way to represent this with
Value<String>
since the valuewasn't actually stored in memory but rather the pointer/length pair is
received as arguments. Overall this meant that
Value<T>
couldn't beused for arguments-to-imports, which means that altogether something new
would be required.
This PR completely removes the
Value<T>
andCursor<T>
type in favorof a different implementation. The inspiration from this comes from the
fact that all primitives can be both lifted and lowered into wasm while
it's just some times which can only go one direction. For example
String
can be lowered into wasm but can't be lifted from wasm. Insteadsome sort of "view" into wasm needs to be created during lifting.
One of the realizations from #4039 was that we could leverage
run-time-type-checking to reject static constructions that don't make
sense. For example if an embedder asserts that a wasm function returns a
Rust
String
we can reject that at typechecking time because it'simpossible for a wasm module to ever do that.
The new system of imports/exports in this PR now looks like:
Type-checking takes into accont an
Op
operation which indicateswhether we'll be lifting or lowering the type. This means that we can
allow the lowering operation for
String
but disallow the liftingoperation. While we can't statically rule out an embedder saying that
a component returns a
String
we can now reject it at runtime anddisallow it from being called.
The
ComponentValue
trait now sports a newload
function. Thisfunction will load and instance of
Self
from the byte-arrayprovided. This is implemented for all types but only ever actually
executed when the
lift
operation is allowed during type-checking.The
Lift
associated type is removed since it's now expected that thelift operation returns
Self
.The
ComponentReturn
trait is now no longer necessary and is removed.Instead returns are bounded by
ComponentValue
. During type-checkingit's required that the return value can be lifted, disallowing, for
example, returning a
String
or&str
.With
Value
gone there's no need to specify the ABI details of thereturn value, or whether it's communicated through memory or not. This
means that handling return values through memory is transparently
handled by Wasmtime.
Validation is in a sense more eagerly performed now. Whenever a value
T
is loaded the entire immediate structure ofT
is loaded andvalidated. Note that recursive through memory validation still does
not happen, so the contents of lists or strings aren't validated, it's
just validated that the pointers are in-bounds.
Overall this felt like a much clearer system to work with and should be
much easier to integrate with imported functions as well. The new
WasmStr
andWasmList<T>
types can be used in import arguments andlifted from the immediate arguments provided rather than forcing them to
always be stored in memory.