Stream: git-wasmtime

Topic: wasmtime / Issue #1565 Reactor support.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 21 2020 at 14:59):

github-actions[bot] commented on Issue #1565:

Subscribe to Label Action

cc @kubkon, @peterhuene

<details>
This issue or pull request has been labeled: "wasi", "wasmtime", "wasmtime:api", "wasmtime:docs"

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

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

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (May 18 2020 at 23:02):

github-actions[bot] commented on Issue #1565:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:c-api"

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

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

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2020 at 14:24):

alexcrichton commented on Issue #1565:

I do like this approach better myself, but I feel like it's still a bit too heavyweight because it makes NewInstance almost exclusively WASI-focused. I would expect Instance to be neutral with respect to WASI-or-not and users have to opt-in to WASI-specific hooks.

I realize it can be a small footgun if these methods are added directly to Instance, but I feel that the footgun is worth it in terms of ergonomics. For example if you're reading the documentation of the wasmtime crate all the examples have init_reactor(&[]) which is somewhat bizarre if you've never heard of WASI before.

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2020 at 20:38):

sunfishcode commented on Issue #1565:

They aren't really WASI-specific. I expect that between the linking spec and interface types, the definition of commands and reactors will eventually want to move out of WASI and into one of those other specs. Would it help if we moved the definition into tool-conventions for now? We could then avoid calling these "WASI" functions. Would that help avoid confusion?

I'm also not tied to the name init_reactor. Would .initialize(&[]) or something else look less surprising?

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2020 at 23:19):

github-actions[bot] commented on Issue #1565:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "fuzzing"

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

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

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2020 at 14:18):

alexcrichton commented on Issue #1565:

Oh sorry, so I mean to say that the current as-is state of NewInstance feels very WASI-specific. The options you have are:

There's not really an easy option for "just give me the Instance I don't care about WASI". Even with init_reactor you have to pass an array of arguments which can be confusing.

This is why I still think it's best to return an work with an Instance. The type-level distinction of a NewInstance I don't think is buying us much here. If we really want we could always probe what an Instance looks like and dynamically guard invocations of functions. We could say, for example, that you can't call WASI functions before you call the wasi initialization function which does the command/reactor thing.

view this post on Zulip Wasmtime GitHub notifications bot (May 22 2020 at 21:59):

sunfishcode commented on Issue #1565:

Ok, I've now gone back to Instance::new not doing any command/reactor stuff. I kept NewInstance, with a start function that now just runs the wasm start function. That also simplifies the C API story, so this patch now has C API updates corresponding to the Rust API updates.

The new approach now adds a new function Linker::module, which handles instantiating the module, automatically handling command/reactor semantics, which ends up being a lot nicer because we don't need APIs that conditionally consume an Instance. Linker::module avoids the user having to deal with Instances altogether.

view this post on Zulip Wasmtime GitHub notifications bot (May 26 2020 at 15:39):

alexcrichton commented on Issue #1565:

Ok this all looks great to me, thanks again for this @sunfishcode! I feel like anything further we can continue to iterate on in-tree if necessary, so I'm gonna merge.

Last updated: Nov 22 2024 at 16:03 UTC