Stream: git-wasmtime

Topic: wasmtime / PR #2719 Redo the statically typed `Func` API


view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2021 at 19:56):

alexcrichton opened PR #2719 from typed-func to main:

This commit reimplements the Func API with respect to statically typed
dispatch. Previously Func had a getN and getN_async family of
methods which were implemented for 0 to 16 parameters. The return value
of these functions was an impl Fn(..) closure with the appropriate
parameters and return values.

There are a number of downsides with this approach that have become
apparent over time:

The main benefit of the previous scheme is that by returning a impl Fn
it was quite easy and ergonomic to actually invoke the function. In
practice, though, examples would often have something akin to
.get0::<()>()?()? which is a lot of things to interpret all at once.
Note that get0 means "0 parameters" yet a type parameter is passed.
There's also a double function invocation which looks like a lot of
characters all lined up in a row.

Overall, I think that the previous design is starting to show too many
cracks and deserves a rewrite. This commit is that rewrite.

The new design in this commit is to delete the getN{,_async} family of
functions and instead have a new API:

impl Func {
    fn typed<P, R>(&self) -> Result<&Typed<P, R>>;
}

impl Typed<P, R> {
    fn call(&self, params: P) -> Result<R, Trap>;
    async fn call_async(&self, params: P) -> Result<R, Trap>;
}

This should entirely replace the current scheme, albeit by slightly
losing ergonomics use cases. The idea behind the API is that the
existence of Typed<P, R> is a "proof" that the underlying function
takes P and returns R. The Func::typed method peforms a runtime
type-check to ensure that types all match up, and if successful you get
a Typed value. Otherwise an error is returned.

Once you have a Typed then, like Func, you can either call or
call_async. The difference with a Typed, however, is that the
params/results are statically known and hence these calls can be much
more efficient.

This is a much smaller API surface area from before and should greatly
simplify the Func documentation. There's still a problem where
Func::wrapN_async produces a lot of functions to document, but that's
now the sole offender. It's a nice benefit that the
statically-typed-async verisons are now expressed with an async
function rather than a function-returning-a-future which makes it both
more efficient and easier to understand.

The type P and R are intended to either be bare types (e.g. i32)
or tuples of any length (including 0). At this time R is only allowed
to be () or a bare i32-style type because multi-value is not
supported with a native ABI (yet). The P, however, can be any size of
tuples of parameters. This is also where some ergonomics are lost
because instead of f(1, 2) you now have to write f.call((1, 2))
(note the double-parens). Similarly f() becomes f.call(()).

Overall I feel that this is a better tradeoff than before. While not
universally better due to the loss in ergonomics I feel that this design
is much more flexible in terms of what you can do with the return value
and also understanding the API surface area (just less to take in).

[old-docs]: https://docs.rs/wasmtime/0.24.0/wasmtime/struct.Func.html#method.get0

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2021 at 19:57):

alexcrichton updated PR #2719 from typed-func to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2021 at 19:57):

alexcrichton edited PR #2719 from typed-func to main:

This commit reimplements the Func API with respect to statically typed
dispatch. Previously Func had a getN and getN_async family of
methods which were implemented for 0 to 16 parameters. The return value
of these functions was an impl Fn(..) closure with the appropriate
parameters and return values.

There are a number of downsides with this approach that have become
apparent over time:

The main benefit of the previous scheme is that by returning a impl Fn
it was quite easy and ergonomic to actually invoke the function. In
practice, though, examples would often have something akin to
.get0::<()>()?()? which is a lot of things to interpret all at once.
Note that get0 means "0 parameters" yet a type parameter is passed.
There's also a double function invocation which looks like a lot of
characters all lined up in a row.

Overall, I think that the previous design is starting to show too many
cracks and deserves a rewrite. This commit is that rewrite.

The new design in this commit is to delete the getN{,_async} family of
functions and instead have a new API:

impl Func {
    fn typed<P, R>(&self) -> Result<&TypedFunc<P, R>>;
}

impl TypedFunc<P, R> {
    fn call(&self, params: P) -> Result<R, Trap>;
    async fn call_async(&self, params: P) -> Result<R, Trap>;
}

This should entirely replace the current scheme, albeit by slightly
losing ergonomics use cases. The idea behind the API is that the
existence of TypedFunc<P, R> is a "proof" that the underlying function
takes P and returns R. The Func::typed method peforms a runtime
type-check to ensure that types all match up, and if successful you get
a TypedFunc value. Otherwise an error is returned.

Once you have a TypedFunc then, like Func, you can either call or
call_async. The difference with a Typed, however, is that the
params/results are statically known and hence these calls can be much
more efficient.

This is a much smaller API surface area from before and should greatly
simplify the Func documentation. There's still a problem where
Func::wrapN_async produces a lot of functions to document, but that's
now the sole offender. It's a nice benefit that the
statically-typed-async verisons are now expressed with an async
function rather than a function-returning-a-future which makes it both
more efficient and easier to understand.

The type P and R are intended to either be bare types (e.g. i32)
or tuples of any length (including 0). At this time R is only allowed
to be () or a bare i32-style type because multi-value is not
supported with a native ABI (yet). The P, however, can be any size of
tuples of parameters. This is also where some ergonomics are lost
because instead of f(1, 2) you now have to write f.call((1, 2))
(note the double-parens). Similarly f() becomes f.call(()).

Overall I feel that this is a better tradeoff than before. While not
universally better due to the loss in ergonomics I feel that this design
is much more flexible in terms of what you can do with the return value
and also understanding the API surface area (just less to take in).

[old-docs]: https://docs.rs/wasmtime/0.24.0/wasmtime/struct.Func.html#method.get0

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2021 at 20:57):

alexcrichton updated PR #2719 from typed-func to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2021 at 21:07):

alexcrichton updated PR #2719 from typed-func to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2021 at 21:39):

alexcrichton updated PR #2719 from typed-func to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2021 at 21:56):

alexcrichton updated PR #2719 from typed-func to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 01:45):

peterhuene submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 01:45):

peterhuene created PR Review Comment:

Given this seems to be common (get_func followed by typed which then needs to be cloned), perhaps we should have Instance::get_typed_func to shorthand this?

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 01:45):

peterhuene submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 01:45):

peterhuene created PR Review Comment:

    /// used instead of [`Func::call`] (which is more general, hence its

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 16:44):

alexcrichton submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 16:44):

alexcrichton created PR Review Comment:

Seems reasonable to me!

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 17:32):

alexcrichton updated PR #2719 from typed-func to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 17:33):

alexcrichton updated PR #2719 from typed-func to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 17:34):

alexcrichton updated PR #2719 from typed-func to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 17:36):

alexcrichton updated PR #2719 from typed-func to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 17:36):

alexcrichton requested peterhuene for a review on PR #2719.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 19:13):

peterhuene submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 19:23):

alexcrichton updated PR #2719 from typed-func to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2021 at 20:43):

alexcrichton merged PR #2719.


Last updated: Oct 23 2024 at 20:03 UTC