wasmtime / PR #7801 c-api/component-model: proof of concept · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #7801 c-api/component-model: proof of concept

Wasmtime GitHub notifications bot (Jan 20 2024 at 14:44):

rockwotj opened PR #7801 from rockwotj:main to bytecodealliance:main:

Zulip link

This is a proof of concept for the component model exposed in the C API. I mostly took the approach of "how would I want this to look in the C API" and let the rust implementation fall out from that. I think there are certainly improvements that could be made to the implementation that I will point out as review comments.

This is a draft to foster discussion, but I don't expect actually merge this.

Wasmtime GitHub notifications bot (Jan 20 2024 at 14:55):

rockwotj submitted PR review.

Wasmtime GitHub notifications bot (Jan 20 2024 at 14:55):

rockwotj submitted PR review.

Wasmtime GitHub notifications bot (Jan 20 2024 at 14:55):

rockwotj created PR review comment:

The Rust API uses the string name as the interface to which discriminant is chosen, personally I think it's more ergonomic in the C-API to use the index, and that's the underlying value stored in wasmtime::component::Val anyways.

Wasmtime GitHub notifications bot (Jan 20 2024 at 14:55):

rockwotj created PR review comment:

The Rust API uses the string name as the interface to which discriminant is chosen, personally I think it's more ergonomic in the C-API to use the index, and that's the underlying value stored in wasmtime::component::Val anyways.

Wasmtime GitHub notifications bot (Jan 20 2024 at 14:55):

rockwotj created PR review comment:

Both the to/from conversion from wasmtime_component_val_t <-> component::Val are both inefficient and awkward. I would like to either expose new_unchecked methods for individual Val subtypes (list, record, etc). These would not do "typechecking", as we're going to have to already do that as we convert. Alternatively, we could upgrade wasmtime_component_val_t into the wasmtime crate as component::RawVal or something and then the conversions could be internal to the crate.

Wasmtime GitHub notifications bot (Jan 20 2024 at 14:55):

rockwotj created PR review comment:

When you mentioned not having good tests for the C-API @alexcrichton can you elaborate what is the preferred way of adding those tests? I see maybe 3 options outside of the "examples" that we have today.

Add in crate unit tests like here, these would be written in Rust, but using the C-API directly, so would feel like a very C style of Rust.

Add in tests to the "integration" style of tests in the tests top level director for the C-API, but written in Rust using this crate directly. There is probably a variant of this where we use bindgen to regenerate Rust bindings from the headers to also test the headers are typed correctly.

Add new "integration" style of tests written in C (or C++) similar to option 2, but just using the binding headers instead of rust directly.

Wasmtime GitHub notifications bot (Jan 20 2024 at 14:55):

rockwotj created PR review comment:

Maybe this should be a format trait implementation?

Wasmtime GitHub notifications bot (Jan 20 2024 at 15:01):

rockwotj edited PR review comment.

Wasmtime GitHub notifications bot (Jan 20 2024 at 16:45):

github-actions[bot] commented on PR #7801:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:api", "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

peterhuene: wasmtime:api, wasmtime:c-api

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Jan 22 2024 at 20:48):

alexcrichton commented on PR #7801:

Thanks for starting on this! I like the idea of the approach here to start with C/C++ APIs and kinda go from there. I want to point out some points though and provide some thoughts on what you've already implemented:

I'd recommend planning out resources pretty early on. They radically change a lot of things if they weren't previously accounted for so you'll want to make sure they're taken care of. (e.g. borrows, owns, etc)

Indices-vs-strings is an interesting question. I think it can go either way, but I additionally think we'll want the ability to inspect type information. For example the enum variant 2 is not the same across different enums. Additionally the enum variant name "foo" also isn't the same across enums. With type information though this can all be equated and the name and/or index can be "resolved" what its relative to.

I'm all for some sort of Val::new_unchecked but I've given it no thought as to what it would look like other than "that should probably exist and I have no idea what it would look like".

Drawing from the parallels of the C API for wasm there's sort of two modes for values - one where a value is tagged with its type and one where a value is "unchecked" and you assert it's the right type. That might be good to follow with components as well where what you've sketched out so far is the untagged/unchecked version so far (I know there's a tag for a kind but it's more of a discriminant than "this is the type of the value"). That'd roughly correspond to TypedFunc and Func in the Rust API sort of.

Wasmtime GitHub notifications bot (Jan 22 2024 at 21:10):

rockwotj commented on PR #7801:

but I additionally think we'll want the ability to inspect type information.

Agreed, I've not spent time on this yet, but was planning on exposing this in a similar manner to wasmtime::component::Func.

two modes for values - one where a value is tagged with its type and one where a value is "unchecked" and you assert it's the right type.

Could you clarify this? I don't quite follow, there seems to only be the ability to call the dynamic function variants (wasmtime::Func) in the C API for wasm. I don't see anything that has the type attached, in fact any method call from host->guest does typechecking today AFAIK.

If you're talking about having TypedFunc support in the C-API, I think that's possible in the component model with some code generation and having a way to write a C struct directly into guest memory using the canonical ABI format.

Wasmtime GitHub notifications bot (Jan 22 2024 at 21:10):

rockwotj edited a comment on PR #7801:

I'd recommend planning out resources pretty early on.

Thanks, yes I need to dig into how they work today a bit before going forward.

but I additionally think we'll want the ability to inspect type information.

Agreed, I've not spent time on this yet, but was planning on exposing this in a similar manner to wasmtime::component::Func.

two modes for values - one where a value is tagged with its type and one where a value is "unchecked" and you assert it's the right type.

Could you clarify this? I don't quite follow, there seems to only be the ability to call the dynamic function variants (wasmtime::Func) in the C API for wasm. I don't see anything that has the type attached, in fact any method call from host->guest does typechecking today AFAIK.

If you're talking about having TypedFunc support in the C-API, I think that's possible in the component model with some code generation and having a way to write a C struct directly into guest memory using the canonical ABI format.

Wasmtime GitHub notifications bot (Jan 22 2024 at 21:13):

alexcrichton commented on PR #7801:

Oh you are correct, no TypedFunc in the C API, I have no idea how that would work. I mean moreso wasmtime_func_call vs wasmtime_func_call_unchecked which is Func::call vs Func::call_unchecked. The latter isn't typed but additionally is unsafe since you assert that whatever you passed is the correct type.

The component model is trickier in this regard where it's not as obvious how a list of records with variants of strings should be laid out in memory (compared to a i32). One idea I've toyed with is that the "raw" version of the component model is the canonical ABI, not a structured value. That would put quite a burden on users of the component model, however, perhaps too much.

Wasmtime GitHub notifications bot (Jan 22 2024 at 21:25):

rockwotj commented on PR #7801:

That would put quite a burden on users of the component model, however, perhaps too much.

Maybe, I mean there would have to be a codegen portion to think about here too. I'm not really sure how passing values back would work in this model.

Reguardless, I think that can come in a v2 and I'm going to focus on just supporting the wasmtime_func_call equivalent for the component model. The unchecked variant could be added for better performance if someone runs into it, but probably the better solution is to invest in figuring out how the TypedFunc variant would work in the C-API.

Wasmtime GitHub notifications bot (Jan 22 2024 at 21:37):

alexcrichton commented on PR #7801:

Makes sense yeah. In that sense I'd recommend ensuring that type information can be tagged on values at the same time, and that'll help a lot with resources and resolve the string-vs-index problem as well (aka always use an index and then the index is relative to what's in the type info)

Wasmtime GitHub notifications bot (Feb 29 2024 at 12:34):

rockwotj closed without merge PR #7801.

Wasmtime GitHub notifications bot (Mar 29 2024 at 19:51):

eighty4 commented on PR #7801:

Curiousity: why doesn't C and Golang use a static lib built from the Rust crate?

Wasmtime GitHub notifications bot (Mar 29 2024 at 19:51):

eighty4 edited a comment on PR #7801:

Curiosity: why doesn't C and Golang use a static lib built from the Rust crate?

Wasmtime GitHub notifications bot (Mar 29 2024 at 20:04):

rockwotj commented on PR #7801:

Curiosity: why doesn't C and Golang use a static lib built from the Rust crate?

It does (from the C API) - however all these languages don't have the same ABI, so C is common set of ABI all these languages speak

Wasmtime GitHub notifications bot (Mar 29 2024 at 20:04):

rockwotj edited a comment on PR #7801:

Curiosity: why doesn't C and Golang use a static lib built from the Rust crate?

It does (from the C API crate) - however all these languages don't have the same ABI, so C is common set of ABI all these languages speak

Last updated: Nov 22 2024 at 17:03 UTC