Stream: git-wasmtime

Topic: wasmtime / issue #6821 wasi-nn: refactor to allow `previe...

Wasmtime GitHub notifications bot (Aug 10 2023 at 15:49):

alexcrichton commented on issue #6821:

One part of this is that it defines a hierarchy of types and then provides conversion of WIT and WITX-generated types into this hierarchy of types. That's reasonable for this crate since the set of types is quite small, but for something like wasi-common I'm not sure if that would work well (or larger proposals). In that sense it might make more sense to unconditionally generate WIT bindings (e.g. not a compile time feature) and always use the WIT types generated? That way there'd only be one conversion necessary which is WITX-to-WIT.

Also one thing to consider is that this has impl WasiNnTrait for WasiNnCtx which means that it's not going to be easily usable from embedders. Currently with bindgen! we're using something that looks like impl<T> WasiNnTrait for T where T: WasiNnView so that way if WasiNnCtx is embedded in a store somewhere the WasiNnView defines how to access it.

Also as a final thing, it looks like the implementation bodies of WIT and WITX are duplicated? Would it be possible for one to delegate to the other?

Wasmtime GitHub notifications bot (Aug 10 2023 at 22:39):

abrown commented on issue #6821:

In that sense it might make more sense to unconditionally generate WIT bindings (e.g. not a compile time feature) and always use the WIT types generated

Yeah, I would prefer that. Is it fine if wasmtime-wasi-nn enables the component-model feature, though, forcing every user of this (e.g., the CLI) to do so too?

impl<T> WasiNnTrait for T where T: WasiNnView

I saw this over in wasi or wasi-common and didn't really "get" it. Let me look into it a bit more.

Also as a final thing, it looks like the implementation bodies of WIT and WITX are duplicated? Would it be possible for one to delegate to the other?

They are duplicated in essence but not all the details are the same; e.g., anywhere we need a slice in witx.rs we do the Wiggle GuestSlice dance. I'll look into this a bit more...

Wasmtime GitHub notifications bot (Aug 10 2023 at 22:58):

abrown commented on issue #6821:

@alexcrichton: one more thought about your comments is that perhaps some of these improvements could be done as a separate PR. There is clearly more work that needs to be done here (e.g., replace all these temporary WIT files with their upstream versions, implement named models, etc.) so perhaps some of the refactorings you suggest could fit into some of those PRs.

Last updated: Nov 22 2024 at 17:03 UTC