wasmtime / issue #4922 Storing Memory Exported By Wasm Mo... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #4922 Storing Memory Exported By Wasm Mo...

Wasmtime GitHub notifications bot (Sep 18 2022 at 03:06):

AldaronLau opened issue #4922:

Feature

Ability to import memory before sending state to wasmtime API.

Benefit

Prevent having to use an Option or MaybeUninit in the state to store Memory in state when it's always going to be Some or initialized by the time a function call from WASM happens.

Implementation

Most likely would require postponing when the state is shared with wasmtime (so the memory can be imported from the wasm module and stored into a state). This could occur after instantiating, possibly with another type.

Alternatives

Keep using Option or MaybeUninit

Possibly change Func::call so that it needs to manually pass in the user state as an additional parameter

Wasmtime GitHub notifications bot (Sep 18 2022 at 07:12):

bjorn3 commented on issue #4922:

Instantiating needs the state as it calls the start function of the wasm module, which can call back to the host functions that use the state AFAIK.

Wasmtime GitHub notifications bot (Sep 19 2022 at 14:16):

alexcrichton commented on issue #4922:

There's a circular dependency here where the Store needs to be created to instantiate a module, which needs the store's data. Your embedder data needs the instantiated module's memory, however, hence the two depend on each other. Unfortunately I'm not sure if there's anything that can be done here.

Possibly change Func::call so that it needs to manually pass in the user state as an additional parameter

This is effectively already done with the passing of the equivalent of &mut Store<T> into this function.

Wasmtime GitHub notifications bot (Sep 20 2022 at 03:31):

AldaronLau commented on issue #4922:

Thanks, that makes sense.

Would it be possible to add an API to wasmtime similar to wasmi::InstancePre::ensure_no_start(), which additionally would not require the user provided state until after instantiation?

Or considering this from the WASM spec:

The component of a module declares the function index of a start function that is automatically invoked when the module is instantiated, after tables and memories have been initialized.

Since memories and tables are initialized before the start function is called, would it be possible to split into a two-stage initialization? I'm assuming memories and tables wouldn't require the user's state, and that the state would only be required right before the invocation of the start function.

Wasmtime GitHub notifications bot (Sep 21 2022 at 14:23):

alexcrichton commented on issue #4922:

While that could be theoretically done I don't think I understand fully why we would do that. Is having an Option onerous to represent the pre-instantiated and post-instantiated state?

Wasmtime GitHub notifications bot (Sep 21 2022 at 14:59):

AldaronLau commented on issue #4922:

My specific use case can be very I/O bound, so if the Option can be removed that's possibly around 1000+ branches in unwraps / second (for calls back to the host, all of which need to read/write the memory exported by the wasm module). It would help for both battery life and efficiency, if it can be safely proved at compile time that it would be initialized.

Wasmtime GitHub notifications bot (Sep 21 2022 at 15:07):

bjorn3 commented on issue #4922:

The branch will likely be correctly predicted in the fast majority of the cases, which means that the overhead is very small. The overhead to call from wasm into host code is much bigger.

Wasmtime GitHub notifications bot (Sep 21 2022 at 15:14):

AldaronLau commented on issue #4922:

Maybe it's not as big of a deal as I was initially thinking. I would just prefer to move checks from runtime to compile time if possible.

Wasmtime GitHub notifications bot (Sep 21 2022 at 15:28):

alexcrichton commented on issue #4922:

What you're proposing is effectively a major new feature and change to the API of instantiation and how embedder state is managed. I would personally expect an appropriate level of motivation to be required for such a change, so I would recommend measuring performance impact and differences to see if it matters for your use case.

My personal intuition is that a few .unwrap() statements on internal state will never really show up in a profile.

Wasmtime GitHub notifications bot (Dec 02 2022 at 00:01):

alexcrichton commented on issue #4922:

I'm going to close this as I think it was answered above.

Wasmtime GitHub notifications bot (Dec 02 2022 at 00:01):

alexcrichton closed issue #4922:

Feature

Ability to import memory before sending state to wasmtime API.

Benefit

Prevent having to use an Option or MaybeUninit in the state to store Memory in state when it's always going to be Some or initialized by the time a function call from WASM happens.

Implementation

Most likely would require postponing when the state is shared with wasmtime (so the memory can be imported from the wasm module and stored into a state). This could occur after instantiating, possibly with another type.

Alternatives

Keep using Option or MaybeUninit

Possibly change Func::call so that it needs to manually pass in the user state as an additional parameter

Wasmtime GitHub notifications bot (Nov 07 2024 at 08:00):

Lohann commented on issue #4922:

Hello, can we reopen this issue? I had a lot of trouble and work to get Wasmtime store working with MaybeUnit, this simple solution segfaults:
pub struct State {
    pub memory: Memory,
}

let state = MaybeUninit::<State>::uninit();
let mut store = Store::new(engine, state);

let memory_type = MemoryType::new(16, None);
store.data_mut().memory.write(Memory::new(&mut store, memory_type)?);

let store = unsafe {
    std::mem::transmute::<Store<MaybeUninit<State>>, Store<State>>(store)
};

// segfaults below
let instance = Instance::new(&mut store, &module, &imports)?;
Then I realized the issue is that there's no way for me transmute only the State, I need to transmute the Store which is not recommended, once rust doesn't guarantee the same memory layout:
assert_eq!(size_of::<Option<bool>>(), 1);
assert_eq!(size_of::<Option<MaybeUninit<bool>>>(), 2);
Actually I haven't find any way to use MaybeUnit that doesn't look hacky, and I want to avoid the usage of Option and unwraps in the code, once it bloats the binary with panic data.

Wasmtime GitHub notifications bot (Nov 07 2024 at 15:46):

alexcrichton commented on issue #4922:

It's best to open a new issue for questions like this, as I see you've done on https://github.com/bytecodealliance/wasmtime/issues/9579, so I'll answer over there.

Last updated: Nov 22 2024 at 17:03 UTC