AldaronLau opened issue #4922:
Feature
Ability to import memory before sending state to wasmtime API.
Benefit
Prevent having to use an
Option
orMaybeUninit
in the state to storeMemory
in state when it's always going to beSome
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
orMaybeUninit
Possibly change
Func::call
so that it needs to manually pass in the user state as an additional parameter
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.
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.
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.
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?
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.
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.
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.
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.
alexcrichton commented on issue #4922:
I'm going to close this as I think it was answered above.
alexcrichton closed issue #4922:
Feature
Ability to import memory before sending state to wasmtime API.
Benefit
Prevent having to use an
Option
orMaybeUninit
in the state to storeMemory
in state when it's always going to beSome
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
orMaybeUninit
Possibly change
Func::call
so that it needs to manually pass in the user state as an additional parameter
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 theStore
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
andunwraps
in the code, once it bloats the binary with panic data.
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: Dec 23 2024 at 12:05 UTC