Stream: git-wasmtime

Topic: rfcs / Issue #5 RFC: Add instance allocator support to Wa...

Wasmtime GitHub notifications bot (Nov 30 2020 at 22:55):

peterhuene commented on Issue #5:

@alexcrichton how awful would it be for Config to expose a method taking an Arc<dyn wasmtime_runtime::InstanceAllocator>? A complete non-starter?

This asymmetry between the InstanceAllocator traits in wasmtime and wasmtime-runtime necessitate a back pointer from wasmtime_runtime::Instance to the allocator that created it so that the instance can call into the allocator upon deallocation of the InstanceHandle. I would really like to see the back pointer removed.

Having Config directly take the runtime trait would both simplify the code (i.e. no duplicated types in the wasmtime crate) and eliminate the problem because Store could use the runtime trait directly to deallocate the instance.

This also means that if we open up the construction of Instance and InstanceHandle in the runtime so that instance allocators can be implemented in external crates, the Wasmtime API can use those implementations without introducing a wrapping type in the wasmtime crate.

Wasmtime GitHub notifications bot (Nov 30 2020 at 23:05):

alexcrichton commented on Issue #5:

Heh I always start with the idea that anything is possible!

More to the point though a move like that would basically just be about the costs. It would cost us in terms of stability that we would have to document that usage of the method is not covered under our semver stability promises. That's probably feasible to do since we don't expect every user to have to implement this? Especially if we can bundle "common" allocation strategies in the wasmtime crate which are covered under the semver rules.

Other than that the only other alternative I can think of is to sync up on the code you've got and see if anyone else is struck with inspiration of how to not have the dependency

Wasmtime GitHub notifications bot (Nov 30 2020 at 23:40):

fitzgen commented on Issue #5:

It wouldn't solve the duplication issue, but would solve the API stability issues: we could create a wasmtime-traits crate that defines just the InstanceAllocator trait and necessary types (e.g. InstanceRequest) and say it has the same stability that the wasmtime crate does (even re-export it from wasmtime).

Wasmtime GitHub notifications bot (Dec 01 2020 at 00:25):

peterhuene commented on Issue #5:

What if we kept the sole InstanceAllocator trait in wasmtime_runtime where it would inherently be unstable (and thus implementors would know that their implementations are subject to breaks in wasmtime_runtime) but we would then consider Config::with_instance_allocator to itself be stable despite taking Arc<dyn wasmtime_runtime::InstanceAllocator> as an argument? The wasmtime crate and any implementors of InstanceAllocator must agree on the definition anyway, right?

The "out of the box" instance allocator implementations could be exported via the wasmtime crate and thus considered stable as their only public API surface are their constructors.

We generally don't expect users to implement the trait themselves as it requires a lot of internal runtime understanding to do so, but we do want an easy mechanism for users to swap out the allocator implementations easily using the public Wasmtime API.

Wasmtime GitHub notifications bot (Dec 01 2020 at 00:28):

peterhuene edited a comment on Issue #5:

What if we kept the sole InstanceAllocator trait in wasmtime_runtime where it would inherently be unstable (and thus implementors would know that their implementations are subject to breaks in wasmtime_runtime) but we would then consider Config::with_instance_allocator to itself be stable despite taking Arc<dyn wasmtime_runtime::InstanceAllocator> as an argument? The wasmtime crate and any implementors of InstanceAllocator must agree on the definition anyway, right?

The "out of the box" instance allocator implementations could be exported via the wasmtime crate and thus considered stable as their only public API surface are their constructors.

We generally don't expect users to implement the trait themselves as it requires a lot of internal runtime understanding to do so, but we do want an easy mechanism for users to swap out the allocator implementations using the public Wasmtime API.

Wasmtime GitHub notifications bot (Dec 01 2020 at 00:30):

fitzgen commented on Issue #5:

If we don't expect anyone else to implement this trait, then we might as well make it an enum at the wasmtime API level, and not expose its guts, types, and implementation details. This would let us write the instance pool in its own (internal) crate, preserve API stability, and avoid duplicating types.

Wasmtime GitHub notifications bot (Dec 01 2020 at 00:31):

fitzgen edited a comment on Issue #5:

If we don't expect anyone else to implement this trait, then we might as well make it an enum InstanceAllocationStategy { Default, Pool } at the wasmtime API level, and not expose its guts, types, and implementation details. This would let us write the instance pool in its own (internal) crate, preserve API stability, and avoid duplicating types.

Wasmtime GitHub notifications bot (Dec 01 2020 at 00:33):

peterhuene commented on Issue #5:

I can get behind that. We could always extend any such enumeration for Custom if a trait is needed in the future.

Wasmtime GitHub notifications bot (Dec 01 2020 at 02:47):

peterhuene commented on Issue #5:

I've updated the RFC with an enum approach that hides the implementation details from the wasmtime crate.

Last updated: Nov 22 2024 at 17:03 UTC