Stream: git-wasmtime

Topic: wasmtime / PR #7804 feat: support component type introspe...

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2024 at 20:24):

rvolosatovs edited PR #7804.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2024 at 20:25):

rvolosatovs edited PR #7804:

This allows for embedders to introspect the component type to e.g. construct Vals of complex types expected by the components at runtime.
Closes #7726
I'll work on adding a simple test once I verify this covers my own use case

cc @alexcrichton

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2024 at 20:28):

rvolosatovs commented on PR #7804:

@alexcrichton I've studied the wasmtime_environ implementation a little bit and looks like I came up with a solution that makes sense (after all types are resolved, "synthesize" a TypeComponent and store in ComponentTypes on finish).

I've added a test case reusing the dynamic::everything (it does not actually contain everything, but hopefully this is "good enough"). PTAL

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2024 at 20:28):

rvolosatovs has marked PR #7804 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2024 at 20:28):

rvolosatovs requested alexcrichton for a review on PR #7804.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2024 at 20:28):

rvolosatovs requested wasmtime-core-reviewers for a review on PR #7804.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2024 at 20:29):

rvolosatovs edited a comment on PR #7804:

@alexcrichton I've studied the wasmtime_environ implementation a little bit and looks like I came up with a solution that makes sense (after all types are resolved, "synthesize" a TypeComponent and store in ComponentTypes on finish).

I've added a test case reusing a big chunk of the dynamic::everything component (it does not actually contain everything, but hopefully this is "good enough"). PTAL

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2024 at 21:19):

rvolosatovs edited a comment on PR #7804:

@alexcrichton I've studied the wasmtime_environ implementation a little bit and looks like I came up with a solution that makes sense (after all types are resolved, "synthesize" a TypeComponent and store in ComponentTypes on finish to acquire a type index, which is then propagated further).
I had to also add the type index info to exports, which was missing.

I've added a test case reusing a big chunk of the dynamic::everything component (it does not actually contain everything, but hopefully this is "good enough"). PTAL

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2024 at 21:44):

rvolosatovs updated PR #7804.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 16:18):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 16:18):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 16:18):

alexcrichton created PR review comment:

This is the main thing that I'm worried about with this PR right now. This means that if a component exports a locally-defined core wasm module and/or a "bag of items" instance then wasmtime will lie and not actually report the export from the type. I think it should be possible to implement this though with some more type-threading-around, however. All the types of all intermediate values are present and accounted for during validation, we just need to pluck it out at the right time and thread it through the system to show up here.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 25 2024 at 12:32):

rvolosatovs updated PR #7804.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 25 2024 at 12:39):

rvolosatovs updated PR #7804.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 25 2024 at 12:43):

rvolosatovs commented on PR #7804:

@alexcrichton although in our conversation yesterday we discussed that the static module and "instance item bag" case type indexes should be constructed somewhere along the way and thrown out, that did not seem to be the case in all occurrences, therefore in the proposed approach the static module type is "synthesized" from compiled artifacts on finish and for cases where instance type is not assigned yet, that is also done recursively in the same step.
Note that updated test case - I duplicated, for the most part, test cases that were causing panics during development due to type not being available (in particular, the "instance item bag" case)

view this post on Zulip Wasmtime GitHub notifications bot (Jan 25 2024 at 12:43):

rvolosatovs edited a comment on PR #7804:

@alexcrichton although in our conversation yesterday we discussed that the static module and "instance item bag" case type indexes should be constructed somewhere along the way and thrown out, that did not seem to be the case in all occurrences, therefore in the proposed approach the static module type is "synthesized" from compiled artifacts on finish for cases where instance type is not assigned yet, that is also done recursively in the same step.
Note that updated test case - I duplicated, for the most part, test cases that were causing panics during development due to type not being available (in particular, the "instance item bag" case)

view this post on Zulip Wasmtime GitHub notifications bot (Jan 25 2024 at 12:44):

rvolosatovs submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 25 2024 at 12:44):

rvolosatovs created PR review comment:

It appears that we can pluck most of these out, but not for all cases, see updated implementation

view this post on Zulip Wasmtime GitHub notifications bot (Jan 25 2024 at 12:44):

rvolosatovs requested alexcrichton for a review on PR #7804.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 25 2024 at 12:49):

rvolosatovs updated PR #7804.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 25 2024 at 12:57):

rvolosatovs submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 25 2024 at 12:57):

rvolosatovs created PR review comment:

@alexcrichton I'm getting a bit lost here, is ty here the correct type index or is it actually the parent instance type index?

view this post on Zulip Wasmtime GitHub notifications bot (Jan 26 2024 at 11:04):

rvolosatovs deleted PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 26 2024 at 19:14):

rvolosatovs edited a comment on PR #7804:

@alexcrichton although in our conversation yesterday we discussed that the static module and "instance item bag" case type indexes should be constructed somewhere along the way and thrown out, that did not seem to be the case in all occurrences, therefore in the proposed approach the static module type is "synthesized" from compiled artifacts on finish for cases where instance type is not assigned yet, that is also done recursively in the same step.
Note the updated test case - I duplicated, for the most part, test cases that were causing panics during development due to type not being available (in particular, the "instance item bag" case)

view this post on Zulip Wasmtime GitHub notifications bot (Jan 26 2024 at 20:02):

alexcrichton submitted PR review:

Looks great to me, thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Jan 26 2024 at 20:49):

alexcrichton merged PR #7804.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 28 2024 at 18:15):

sehz commented on PR #7804:

Nice work!

Last updated: Dec 23 2024 at 13:07 UTC