alexcrichton edited Issue #708:
I've been reviewing the
wasmtime
crate from a Rust API perspective and ended up realizing that there's actually quite a few changes that I would like to make to the crate. I think that these changes are far too large so simply send in a PR, so I wanted to make sure that we had some discussion of this first!In this issue I hope to lay out a vision for an end-state
wasmtime
crate and what the API might look like. I'm assuming that we can incrementally reach this end-goal over time and the exact route through which we get here isn't too too important. In any case, I'm curious if others have thoughts on all this!Some of these items below may warrant their own separate issue as well, but I wanted to make sure that I had this all written down in one location first
High-level changes
- All
wasmtime-*
crates should be private dependenciesStore
should implementDefault
- Document everything
- Update
Send
andSync
of all types- Internal reference counting instead of
HostRef
,HostRef
is hidden- No methods should use raw identifiers
Instance
is no longer reference counted to ensure that it'sSend
meaning globals/tables/functions are no longer reference counted either.
Config
- [x] Should remove methods referencing foreign
wasmtime-*
types- [x] Should assert
Send
andSync
Engine
- [x] Should implement
Default
(uses defaultConfig
)- [x] Should assert
Send
andSync
- [x] Should internally reference count (implements
Clone
)
Store
- [x] Should implement
Default
(uses defaultConfig
andEngine
)- [x]
Should be(moved to https://github.com/bytecodealliance/wasmtime/issues/777)Send
andSync
- [x] Should internally reference count (implements
Clone
)- [ ] Remove
wasmtime-*
accessors
Module
- [x]
Should be(covered by https://github.com/bytecodealliance/wasmtime/issues/777)Send
andSync
- [x] Should internally reference count (implements
Clone
)- [x] Expose a
&Store
accessor- [x] Implementation-wise constructor should likely compile the wasm code
Instance
- [x] Constructor shouldn't require
Store
(inferred fromModule
)- [x] Constructor's return should expose
Trap
(handled ifTrap
is anError
)- [x] Should expose
module
andstore
accessor- [x]
Should be(moved to https://github.com/bytecodealliance/wasmtime/issues/793)Send
- Internally not reference counted
- Accessing tables/etc all return a reference, not an owned type
- [x]
find_export_by_name
=>get_export
Global
,Table
,Memory
- [x] Remove all internal reference counts
Memory
should be fine to leave with an internalArc
- [x]
Everything should be(moved to https://github.com/bytecodealliance/wasmtime/issues/793)Send
, onlyMemory
should beSync
- [x]
Table::grow
shouldn't needmut
- [x]
Global::set
shouldn't needmut
- [x]
type
methods renamed toty
- [x]
Memory::grow
shouldn't needmut
These types are only accessed via a reference when fetched through an
Instance
.
Func
This type I think needs a lot of improvements. I think it's best to separate out these concerns into a separate issue, however. Some high-level unbaked thoughts are:
- Right now it's very allocation-heavy, probably too much so
- There should be a way of taking a function pointer (as a
usize
) and registering it with a type signature. This would skip all trampolines/etc and we'd document the expected ABI. This would be anunsafe
method.- There should also be a method of extracting an internal function pointer with the right ABI. You would then manually assert the function type as necessary and would unsafely transmute the function pointer returned to a function pointer of the right type.
- Somehow data payloads are handled for the above "raw" methods, haven't fully thought this through...
MemoryType
,TableType
,GlobalType
- Bikeshed on the name
Mutability
, otherwise seems good!
ImportType
,ExportType
- a little allocation-heavy, but probably fine
FuncType
- Feels a bit allocation-heavy right now
- Maybe intern in a
Store
, but probably fine for now.
Trap
- [x] Implement the
Error
trait- [x] Ensure it's pointer-sized
- [x] Ensure it's
Clone
,Send
, andSync
Val
- Commit to "ownership semantics"
AnyRef
will become some custom form ofRc
defined bywasmtime
itself
- jit will have its own refcounting management for tables/globals/etc (needs to be implemented)
- Let's do
Box<u128>
for now to avoid blowing up the size- Enusring "ownership semantics" leaves room for implementing interface types strings eventually
- Consider making
Val
an entirely opaque struct, perhaps with adecode
method which returns a fullenum
. We may not want to commit to a public byte-by-byte representation of aVal
just yet. This seems like an advanced concern we can punt though.WASI
This is now split off to https://github.com/bytecodealliance/wasmtime/issues/727
- Somehow we need to add WASI support to the
wasmtime
crate- "bare minimum" is
fn wasi_unstable(&WasiConfiguration) -> HashMap<String, Extern>
- Perhaps related to the name resolution point below?
* Or perhaps create anInstance
somehow?Name Resolution
This is now split off to https://github.com/bytecodealliance/wasmtime/issues/727
Instantiation is a bit wonky where you have to line up imports 1:1 with the expected imports of the module. We should explore ideas where we have a more name resolution based mechanism which leverages the module system. Would perhaps make it much easier to slot in WASI or slot in a module. Pretty tricky API though so we'd have to think elsewhere about this.
alexcrichton edited Issue #708:
I've been reviewing the
wasmtime
crate from a Rust API perspective and ended up realizing that there's actually quite a few changes that I would like to make to the crate. I think that these changes are far too large so simply send in a PR, so I wanted to make sure that we had some discussion of this first!In this issue I hope to lay out a vision for an end-state
wasmtime
crate and what the API might look like. I'm assuming that we can incrementally reach this end-goal over time and the exact route through which we get here isn't too too important. In any case, I'm curious if others have thoughts on all this!Some of these items below may warrant their own separate issue as well, but I wanted to make sure that I had this all written down in one location first
High-level changes
- All
wasmtime-*
crates should be private dependenciesStore
should implementDefault
- Document everything
- Update
Send
andSync
of all types- Internal reference counting instead of
HostRef
,HostRef
is hidden- No methods should use raw identifiers
Instance
is no longer reference counted to ensure that it'sSend
meaning globals/tables/functions are no longer reference counted either.
Config
- [x] Should remove methods referencing foreign
wasmtime-*
types- [x] Should assert
Send
andSync
Engine
- [x] Should implement
Default
(uses defaultConfig
)- [x] Should assert
Send
andSync
- [x] Should internally reference count (implements
Clone
)
Store
- [x] Should implement
Default
(uses defaultConfig
andEngine
)- [x]
Should be(moved to https://github.com/bytecodealliance/wasmtime/issues/777)Send
andSync
- [x] Should internally reference count (implements
Clone
)- [x] Remove
wasmtime-*
accessors
Module
- [x]
Should be(covered by https://github.com/bytecodealliance/wasmtime/issues/777)Send
andSync
- [x] Should internally reference count (implements
Clone
)- [x] Expose a
&Store
accessor- [x] Implementation-wise constructor should likely compile the wasm code
Instance
- [x] Constructor shouldn't require
Store
(inferred fromModule
)- [x] Constructor's return should expose
Trap
(handled ifTrap
is anError
)- [x] Should expose
module
andstore
accessor- [x]
Should be(moved to https://github.com/bytecodealliance/wasmtime/issues/793)Send
- Internally not reference counted
- Accessing tables/etc all return a reference, not an owned type
- [x]
find_export_by_name
=>get_export
Global
,Table
,Memory
- [x] Remove all internal reference counts
Memory
should be fine to leave with an internalArc
- [x]
Everything should be(moved to https://github.com/bytecodealliance/wasmtime/issues/793)Send
, onlyMemory
should beSync
- [x]
Table::grow
shouldn't needmut
- [x]
Global::set
shouldn't needmut
- [x]
type
methods renamed toty
- [x]
Memory::grow
shouldn't needmut
These types are only accessed via a reference when fetched through an
Instance
.
Func
This type I think needs a lot of improvements. I think it's best to separate out these concerns into a separate issue, however. Some high-level unbaked thoughts are:
- Right now it's very allocation-heavy, probably too much so
- There should be a way of taking a function pointer (as a
usize
) and registering it with a type signature. This would skip all trampolines/etc and we'd document the expected ABI. This would be anunsafe
method.- There should also be a method of extracting an internal function pointer with the right ABI. You would then manually assert the function type as necessary and would unsafely transmute the function pointer returned to a function pointer of the right type.
- Somehow data payloads are handled for the above "raw" methods, haven't fully thought this through...
MemoryType
,TableType
,GlobalType
- Bikeshed on the name
Mutability
, otherwise seems good!
ImportType
,ExportType
- a little allocation-heavy, but probably fine
FuncType
- Feels a bit allocation-heavy right now
- Maybe intern in a
Store
, but probably fine for now.
Trap
- [x] Implement the
Error
trait- [x] Ensure it's pointer-sized
- [x] Ensure it's
Clone
,Send
, andSync
Val
- Commit to "ownership semantics"
AnyRef
will become some custom form ofRc
defined bywasmtime
itself
- jit will have its own refcounting management for tables/globals/etc (needs to be implemented)
- Let's do
Box<u128>
for now to avoid blowing up the size- Enusring "ownership semantics" leaves room for implementing interface types strings eventually
- Consider making
Val
an entirely opaque struct, perhaps with adecode
method which returns a fullenum
. We may not want to commit to a public byte-by-byte representation of aVal
just yet. This seems like an advanced concern we can punt though.WASI
This is now split off to https://github.com/bytecodealliance/wasmtime/issues/727
- Somehow we need to add WASI support to the
wasmtime
crate- "bare minimum" is
fn wasi_unstable(&WasiConfiguration) -> HashMap<String, Extern>
- Perhaps related to the name resolution point below?
* Or perhaps create anInstance
somehow?Name Resolution
This is now split off to https://github.com/bytecodealliance/wasmtime/issues/727
Instantiation is a bit wonky where you have to line up imports 1:1 with the expected imports of the module. We should explore ideas where we have a more name resolution based mechanism which leverages the module system. Would perhaps make it much easier to slot in WASI or slot in a module. Pretty tricky API though so we'd have to think elsewhere about this.
Last updated: Nov 22 2024 at 17:03 UTC