alexcrichton opened PR #8786 from alexcrichton:export-lookup
to bytecodealliance:main
:
This PR is a series of commits that refactor and reimplement how component exports are modeled in Wasmtime at the API layer. The motivations for this commit are:
- Currently looking up the exports of a component unconditionally requires string lookups at runtime. Unlike with core modules this can't be frontloaded to a pre-instantiation step. While not a huge amount of runtime it's a problem I've wanted to solve historically.
- Currently #8395 is unsolved and will require adding even more work to the export lookup path. This is something I've been hesitant to do without implement the previously mentioned optimization as it would slow down the instantiation process even more since now it'd do semver parsing and such.
- Eventually I'd like to be able to, at the API layer of Wasmtime, reason about the exports of pre-instantiated components to build up a graph of instantiations via
InstancePre
to enable components to call each other. That isn't done in this PR but this is intended to lay some groundwork for that.In the process of solving these motivations I've redesigned how exports work on components. Previously a
Component
only hadcomponent_type()
and anInstance
has a "walker"-style API which could be used to walk through the exports of a component (performing string lookups along the way). AllInstance
-based infrastructure has now been removed and replaced withComponentExportIndex
.A
ComponentExportIndex
represents a preloaded pointer to an export, or where one will be. This can be acquired throughComponent::export_index
which can be done during the compilation/loading process of a component. To handle the nested nature of component instance exports this function also takes anOption<&ComponentExportIndex>
representing the parent instance whereNone
is the root of the component. This enables walking the entire exported API of a component with only this function and no need for a nested traversal with types.Additionally all
Instance::get_*
methods are generalized to take a new generic argument instead of just a&str
. If this argument is a string it's a convenience method to load from the root of the component, but otherwise aComponentExportIndex
can also be passed here. A newInstance::get_export
method can be used to lookupComponentExportIndex
at runtime and traverse through the instance nesting as well (and also takes anOption<&ComponentExportIndex>
argument for the parent in the same way asComponent::export_index
).This all then leads to changes in the output of
bindgen!
. Namely I wanted to take advantage of these new features so the creation of bindgen-generated wrappers is now done with:
- A
*Pre
structure is created fromInstancePre<T>
and wrapsInstancePre<T>
as well as each of theComponentExportIndex
values for all the internal functions.- A
FooPre::instantiate
method exists to create aFoo
where all functions are looked up via index.- A
Foo::instantiate
convenience is still provided to do these two steps internally.The diff here is a bit scarier than the actual size of this PR due to the number of updates to the bindgen-generated output for the wit-bindgen tests. Otherwise though much of this PR is refactoring to handle all the above consequences.
alexcrichton requested elliottt for a review on PR #8786.
alexcrichton requested wasmtime-core-reviewers for a review on PR #8786.
alexcrichton commented on PR #8786:
ping @elliottt, mind taking a look at this? I can also reassign if you'd prefer too
elliottt submitted PR review:
I read through the first three commits in detail, and skimmed the changes to updated files in the last two. This looks like a great change!
elliottt submitted PR review:
I read through the first three commits in detail, and skimmed the changes to updated files in the last two. This looks like a great change!
elliottt created PR review comment:
Does this need to be
str::lookup(self.as_str(), component)
? It's not obvious to me how the instance forstr
will be picked here.
elliottt created PR review comment:
Would
.cloned()
work here?.cloned(),
elliottt edited PR review comment.
elliottt edited PR review comment.
alexcrichton updated PR #8786.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Ah this ends up being a "cute" way to force rustc to do auto-deref without actually doing it yourself. The
str::lookup
function name here is the sugar-y form of<str as InstanceExportLookup>::lookup
where we're explicitly selecting the impl onstr
so rustc knows ahead of time that the first argument must have type&str
. Given the&String
receiver it auto-derefs into&str
.(this only works as
str::lookup
isn't a method otherwise or on any other trait)
alexcrichton has enabled auto merge for PR #8786.
alexcrichton updated PR #8786.
alexcrichton has enabled auto merge for PR #8786.
alexcrichton merged PR #8786.
Last updated: Dec 23 2024 at 12:05 UTC