Merge component-init into wasmtime-wizer#11878
Merge component-init into wasmtime-wizer#11878fitzgen merged 7 commits intobytecodealliance:mainfrom
Conversation
Subscribe to Label Actioncc @fitzgen DetailsThis issue or pull request has been labeled: "wizer"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
|
Oh, another thing not yet implemented -- the init function is not deleted from the component just yet. |
This commit is the next phase of merging the wizer and component-init
repositories into Wasmtime. This does not take the same approach as
merging wizer where the git histories were merged, but instead this
takes an entirely different approach for component-init. Effectively I
read the code in component-init and copied over the spirit of the code
into the wasmtime-wizer crate. Very little was literally copied over due
to such large changes in the internals of organization and
implementation. The main goal of merging this repository is to replace
the core wasm tracking of state in component-init with what
wasmtime-wizer already has. Sort of like the runtime in the `wasmtime`
crate the goal is to build component support entirely on top of core
support to avoid duplicating anything.
The general strategy for pre-init components is that unlike core wasm
where more exports are added a component has a new module and new
accessor functions injected for all state found in the component. For
example an `i32` global results in a `(func (result s32))` in WIT.
Memories result in `(func (result (list u8)))`. This "accessor module"
is synthetically built during the instrumentation pass of Wizer and then
used to acquire snapshot results afterwards.
All of this support is added in a new `component` submodule of the
`wasmtime-wizer` crate. This submodule has the same structure as its
core wasm counterpart at the root of the crate, but the internal
implementations are entirely different. Anything encountering a core
wasm module delegates to the core wasm support in the root of the crate
for Wizer.
There is one new limitation at this time over what component-init
supports which is that nested components are not supported just yet.
Currently in component-init nested components are copied over as-is
which ends up producing a faulty initialization if the components
actually have core wasm instances associated with them. For now this
implementation sidesteps this by forbidding nested components entirely.
To support wizening the output of `wasm-tools component {new,link}`,
however, some support for nested components will be required. I plan on
adding that in a follow-up commit.
Testing of component-init is pretty light right now so this commit
copies over a few `*.wat` tests "in spirit" but does not literally copy
over the preexisting tests. There are a few tests in component-init
which initialize the real output of `wasm-tools component new` which may
want to be migrated eventually but my hope is that this repo can stick
to smaller more focused tests for now.
One large-ish change made to `wasmtime-wizer` during this merge was to
change snapshotting functionality to being an `async` function. This is
required for components because reading state requires invoking a
function, which in the context of the `wasmtime run` subcommand is
always done in "async mode". This meant that the `async` propagates
outwards to much of `wasmtime-wizer`, even the core wasm traits. My hope
is that this isn't a big issue as the CLI can deal with it and embedders
can throw an `async` in there.
Overall this is intended to be a mostly-complete skeleton plus basic
functionality for component-init. I have not done thorough testing with
real-world components just yet (e.g. componentize-py) so there will
likely be follow-up PRs to address various inevitable shortcomings I've
introduced in this merge.
578f83f to
8ea0596
Compare
crates/wizer/src/snapshot.rs
Outdated
| fn merge(&mut self, other: &Self) { | ||
| let gap = self.gap(other); | ||
|
|
||
| DataSegment { | ||
| offset: self.offset, | ||
| len: self.len + gap + other.len, | ||
| ..self.clone() | ||
| } | ||
| self.len += gap + other.len; | ||
| self.data.extend(std::iter::repeat(0).take(gap as usize)); | ||
| self.data.extend_from_slice(&other.data); |
There was a problem hiding this comment.
Hmm... I'm a little worried about some lurking accidental quadratic stuff in here. I don't have time at this very moment, but if you do, could you look at how merge is used below and double check that we don't call merge O(len(data segments)) times? If all looks good, then feel free to merge.
Otherwise, I can take a closer look on Monday.
There was a problem hiding this comment.
That is, maybe the original copying of everythign the once was better than trying to minimize copies but then copying the copies multiple times :-/
There was a problem hiding this comment.
Oh this most definitely is quadratic now that I look at it. I'm going to revert this change in this PR, and I think I see how to do a follow-up PR which only copies out the nonzero portions. Would you be ok landing this to split out that part?
There was a problem hiding this comment.
Would you be ok landing this to split out that part?
Yeah, definitely 👍
This reverts commit 865146c.
This commit is the next phase of merging the wizer and component-init repositories into Wasmtime. This does not take the same approach as merging wizer where the git histories were merged, but instead this takes an entirely different approach for component-init. Effectively I read the code in component-init and copied over the spirit of the code into the wasmtime-wizer crate. Very little was literally copied over due to such large changes in the internals of organization and implementation. The main goal of merging this repository is to replace the core wasm tracking of state in component-init with what wasmtime-wizer already has. Sort of like the runtime in the
wasmtimecrate the goal is to build component support entirely on top of core support to avoid duplicating anything.The general strategy for pre-init components is that unlike core wasm where more exports are added a component has a new module and new accessor functions injected for all state found in the component. For example an
i32global results in a(func (result s32))in WIT. Memories result in(func (result (list u8))). This "accessor module" is synthetically built during the instrumentation pass of Wizer and then used to acquire snapshot results afterwards.All of this support is added in a new
componentsubmodule of thewasmtime-wizercrate. This submodule has the same structure as its core wasm counterpart at the root of the crate, but the internal implementations are entirely different. Anything encountering a core wasm module delegates to the core wasm support in the root of the crate for Wizer.There is one new limitation at this time over what component-init supports which is that nested components are not supported just yet. Currently in component-init nested components are copied over as-is which ends up producing a faulty initialization if the components actually have core wasm instances associated with them. For now this implementation sidesteps this by forbidding nested components entirely. To support wizening the output of
wasm-tools component {new,link}, however, some support for nested components will be required. I plan on adding that in a follow-up commit.Testing of component-init is pretty light right now so this commit copies over a few
*.wattests "in spirit" but does not literally copy over the preexisting tests. There are a few tests in component-init which initialize the real output ofwasm-tools component newwhich may want to be migrated eventually but my hope is that this repo can stick to smaller more focused tests for now.One large-ish change made to
wasmtime-wizerduring this merge was to change snapshotting functionality to being anasyncfunction. This is required for components because reading state requires invoking a function, which in the context of thewasmtime runsubcommand is always done in "async mode". This meant that theasyncpropagates outwards to much ofwasmtime-wizer, even the core wasm traits. My hope is that this isn't a big issue as the CLI can deal with it and embedders can throw anasyncin there.Overall this is intended to be a mostly-complete skeleton plus basic functionality for component-init. I have not done thorough testing with real-world components just yet (e.g. componentize-py) so there will likely be follow-up PRs to address various inevitable shortcomings I've introduced in this merge.