Skip to content
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

component model: async host function & embedding support #5055

Merged
merged 18 commits into from
Oct 18, 2022

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Oct 14, 2022

Based on #5065.

This PR adds the async suite of variations on Linker::wrap_func, Func::call, TypedFunc::call, and the various instantiate functions, for use in async Rust contexts.

This ends up being pretty simple: all of the async store machinery from core wasmtime is straightforward to reuse here.

I created a new test suite at tests/all/component_model/async.rs to smoke test the new functionality.

@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Oct 14, 2022
@github-actions
Copy link

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

  • peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@alexcrichton
Copy link
Member

I've double-checked bits and pieces internally and I think this should generally be good to go. I'm always somewhat wary of async since it's got unsafe underpinnings with fibers and whatnot but I'm at least for now confident enough in this that I'd be happy to merge.

@pchickey
Copy link
Contributor Author

Thanks! I just need to write some tests.

@pchickey pchickey force-pushed the pch/component_model_async branch from 99cefae to b52f4ae Compare October 17, 2022 21:45
Pat Hickey added 7 commits October 17, 2022 15:54
… accepting a closure

We find that this makes the Linker::func_wrap type signature much easier
to read. The IntoComponentFunc abstraction was adding a lot of weight to
"splat" a set of arguments from a tuple of types into individual
arguments to the closure. Additionally, making the StoreContextMut
argument optional, or the Result<return> optional, wasn't very
worthwhile.
@github-actions github-actions bot added the fuzzing Issues related to our fuzzing infrastructure label Oct 18, 2022
@github-actions
Copy link

Subscribe to Label Action

cc @fitzgen

This issue or pull request has been labeled: "fuzzing"

Thus the following users have been cc'd because of the following labels:

  • fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

store.0.async_support(),
"cannot use `call_async` without enabling async support in the config"
);
store.on_fiber(|store| self.post_return_impl(store)).await?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One day in the future it might be best to conditionally do the fiber business here since many functions might not have a post_return in which case going through all this for a stack is largely overkill. Put another way the stack is only needed here for the crate::Func::call_unchecked_raw call, but this is an optimization we can deal with later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted!

Comment on lines 50 to 73
(component
(type $f (func))
(type $imports_type
(instance
(alias outer 1 0 (type $f))
(export "i" (func $f))
)
)
(import "imports" (instance $imports (type $imports_type)))

(core module $m
(import "imports" "i" (func $i))
(func (export "thunk") call $i)
)

(alias export 0 "i" (func $f))
(core func $f_lowered (canon lower (func 0)))
(core instance $imports_core (export "i" (func $f_lowered)))
(core instance $i (instantiate $m
(with "imports" (instance $imports_core))))
(func (export "thunk")
(canon lift (core func $i "thunk"))
)
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simplify this a bit with:

(component
  (import "i" (func $f))

  (core module $m
    (import "" "" (func $i))
    (func (export "thunk") call $i)
  )

  (core func $f (canon lower (func $f)))
  (core instance $i (instantiate $m
    (with "" (instance
      (export "" (func $f))
    ))
  ))
  (func (export "thunk")
    (canon lift (core func $i "thunk"))
  )
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, done!

@pchickey pchickey marked this pull request as ready for review October 18, 2022 16:32
@pchickey pchickey force-pushed the pch/component_model_async branch from e867329 to 2f88f68 Compare October 18, 2022 17:02
@alexcrichton alexcrichton merged commit 12e4a1b into main Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants