alexcrichton requested fitzgen for a review on PR #11878.
alexcrichton requested dicej for a review on PR #11878.
alexcrichton opened PR #11878 from alexcrichton:wizer-component-init to bytecodealliance:main:
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.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
alexcrichton requested wasmtime-core-reviewers for a review on PR #11878.
alexcrichton requested wasmtime-default-reviewers for a review on PR #11878.
alexcrichton updated PR #11878.
pchickey submitted PR review.
github-actions[bot] commented on PR #11878:
Subscribe to Label Action
cc @fitzgen
<details>
This issue or pull request has been labeled: "wizer"Thus the following users have been cc'd because of the following labels:
- fitzgen: wizer
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
alexcrichton commented on PR #11878:
Oh, another thing not yet implemented -- the init function is not deleted from the component just yet.
fitzgen submitted PR review.
fitzgen created PR review comment:
Should the memory accessors be a little more fine-grained? Copying out the whole memory seems like maybe not the foundational primitive we want here. I could imagine a cursor thing to iterate over non-zero regions of memory, or even just returning a
list<(u32, list<u8>)>to skip over zero regions.
fitzgen created PR review comment:
Can you additionally add a test for multiple instantiations of the same core module? I believe this is not currently supported, right? Because it would otherwise require pulling out memories and turning them into imports into the core module, to prevent duplicating code. So just asserting that it fails wizening with a helpful "blah blah is unsupported" message should be good.
fitzgen created PR review comment:
Can we continue to have a default value here, since it shows up in the help text, but just make it the same for components and core modules? It can't be
wizer.initializeanymore because the.means it isn't a valid component export name. Andcomponent-initdoesn't make sense for core modules. How aboutwizer-initorwizer-initialize?
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Right yeah that's not supported. There's a test here for that, but to confirm you aren't thinking of anything more comprehensive than that?
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Technically this is how core wasm sort of works right now anyway where we use
memory.data(&store).to_vec()to get the contents of memory. Building up something likelist<(u32, list<u8>)>would require to write the loop in-wasm and, while possible, would have to handle things like dynamically allocating the outer list and managing the size/length of that. Efficiency-wise we could improve on things here by using theWasmList<u8>type instead ofVec<u8>if this becomes a problem though and that'll enable the host to do the zero-searching without actually copying out the entire memory. There'd be some refactoring of the various internals of that, however.Want me to try and push on such a refactoring and see if it works?
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Would you be ok with the break of compatibility with wizer-from-before by defaulting to
wizer-initializefor core wasm instead ofwizer.initialize?
fitzgen submitted PR review.
fitzgen created PR review comment:
Ah I just missed that test. Thanks!
fitzgen submitted PR review.
fitzgen created PR review comment:
I didn't realize we already copied memories for core Wasm; thought we were using borrows. I guess it isn't a blocker in that case, but if you have cycles to poke at it, seems worth exploring. But not the highest priority, I guess.
fitzgen submitted PR review.
fitzgen created PR review comment:
Yeah, seems fine by me. We are already moving from a standalone CLI to a subcommand of
wasmtime, so seems like an otherwise opportune moment to bundle in little things like this.
alexcrichton updated PR #11878.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Nah easy enough to test, mind taking a look at the latest commit?
alexcrichton updated PR #11878.
fitzgen submitted PR review.
fitzgen created PR review 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
mergeis used below and double check that we don't call mergeO(len(data segments))times? If all looks good, then feel free to merge.Otherwise, I can take a closer look on Monday.
fitzgen submitted PR review.
fitzgen created PR review 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 :-/
alexcrichton submitted PR review.
alexcrichton created PR review 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?
alexcrichton updated PR #11878.
fitzgen submitted PR review.
fitzgen created PR review comment:
Would you be ok landing this to split out that part?
Yeah, definitely :+1:
fitzgen merged PR #11878.
Last updated: Dec 06 2025 at 07:03 UTC