Stream: git-wasmtime

Topic: wasmtime / Issue #2580 Wasmtime: Clarify the relationship...

Wasmtime GitHub notifications bot (Jan 14 2021 at 01:35):

Ekleog edited Issue #2580:

Feature

Automatically call _initialize for reactors, maybe using a special function to create a reactor Module?

Benefit

Knowing one has to call _initialize is something that is not very documented (well… ok, currently nothing is very documented, but seeing as support for wasi reactors landed in rustc like two days ago it's not surprising… anyway my struggles are documented here: https://github.com/rust-lang/rust/pull/79997#issuecomment-759856446)

Implementation

Maybe this would be better with a separate function? But I somehow feel like calling linker.instantiate(&module) automatically calling _initialize would make sense… or maybe add a mandatory argument so people must express the choice of whether they want their module to be initialized at instantiation time or not, and at least know they have to do it manually if they want not to?

Alternatives

Keep the statu quo, ie. let the user call _initialize manually. IMO that would require much more documentation about the topic, though it's definitely a possibility too.

Thank you for all you do on wasm and the surrounding ecosystem! :heart:

Wasmtime GitHub notifications bot (Jan 14 2021 at 01:49):

Ekleog commented on Issue #2580:

Actually I've just been pointed to the fact that Linker::module() already does it. I have just been misusing Linker by assuming it was designed to give out an Instance, instead of to directly give out the exports with get_one_by_name.

As such, let's repurpose this issue for documentation, to avoid that other people : maybe it should be made clear in the documentation that Linkers are there for when the user needs to link multiple modules together, and that often instantiate() should be replaced by module() and get_one_by_name()?

Also… maybe it'd make sense to expose the same convenience getters on Linker as on Instance? The diff of my switch to using Linker::module instead of Linker::instantiate isn't that great :) https://github.com/Ekleog/kannader/commit/ee7de0461f62bb50420503ab7bef98b5e7149165

Wasmtime GitHub notifications bot (Jan 14 2021 at 01:51):

Ekleog edited Issue #2580:

The below feature description is actually already implemented, so I repurposed that issue as a documentation issue, see the second message

Feature

Automatically call _initialize for reactors, maybe using a special function to create a reactor Module?

Benefit

Knowing one has to call _initialize is something that is not very documented (well… ok, currently nothing is very documented, but seeing as support for wasi reactors landed in rustc like two days ago it's not surprising… anyway my struggles are documented here: https://github.com/rust-lang/rust/pull/79997#issuecomment-759856446)

Implementation

Maybe this would be better with a separate function? But I somehow feel like calling linker.instantiate(&module) automatically calling _initialize would make sense… or maybe add a mandatory argument so people must express the choice of whether they want their module to be initialized at instantiation time or not, and at least know they have to do it manually if they want not to?

Alternatives

Keep the statu quo, ie. let the user call _initialize manually. IMO that would require much more documentation about the topic, though it's definitely a possibility too.

Thank you for all you do on wasm and the surrounding ecosystem! :heart:

Wasmtime GitHub notifications bot (Jan 21 2021 at 01:18):

sunfishcode commented on Issue #2580:

Thanks for filing this! I agree, it would be good to clarify the docs in this area.

If I read the diff correctly, the main convenience methods you'd want on Linker are get_func and get_memory, right? The Linker API does want to be careful about cases where there are multiple exports that match—hence the get_one_* naming. But offhand it does seem like accessors dedicated for querying functions etc. would be useful.

Wasmtime GitHub notifications bot (Jan 30 2021 at 10:07):

Ekleog commented on Issue #2580:

Yes, that's exactly it :)

Last updated: Nov 22 2024 at 17:03 UTC