fitzgen opened PR #12500 from fitzgen:type-registry-handle-oom to bytecodealliance:main:
This refactors the type registry to be more "columnar" when registering the types within a rec group, so that we
- Add all the types themselves
- Add the rec group metadata for all types in the rec group
- Add the supertypes metadata for all types in the rec group
- Etc...
Instead of adding one type, its rec group metadata, its supertypes metadata, etc... and then moving on to the next type.
This makes it easier to pre-reserve space and roll back changes on OOM errors.
This is part of https://github.com/bytecodealliance/wasmtime/issues/12069 and the OOM handling effort, but doesn't fully get the
TypeRegistryto a place where it handles all OOMs yet. There are a couple places that need further work (usage of hash sets andCow::into_owned) which I have marked withTODOcomments. In the meantime, I found this to be a nice refactoring of the existing functionality, so I think it can land as-is.<!--
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
-->
fitzgen requested pchickey for a review on PR #12500.
fitzgen requested wasmtime-core-reviewers for a review on PR #12500.
github-actions[bot] added the label wasmtime:api on PR #12500.
alexcrichton submitted PR review:
Once the hashset/
into_ownedbits are handled this'll I think want to be sure to have some tests around registering "interesting shapes" of modules w.r.t. types as the fuzzer + debug assertions should then be pretty good about exploring the space and guaranteeing that any OOM anywhere is gracefully handled
alexcrichton created PR review comment:
Can this comment be moved down to the point where it's "after this we can't use
?" as otherwise it's mildly confusing to read this and then the next few lines all use?.
alexcrichton created PR review comment:
Should this perhaps also debug assert that the capacity of
self.type_to_rec_groupis> ty.index()? Basically refining the debug assert at the start where it's not just bigger thanself.types, but it's a precisely enabled to insertty.
alexcrichton created PR review comment:
Although, alternatively, this is happening in a phase where
?works so would it make sense to avoid dealing with capacities and just early-return on OOM?
fitzgen submitted PR review.
fitzgen created PR review comment:
Yeah, I'll tweak it. Its kinda funky because we do use
?throughout, but we should prefer not to, as much as possible (but will not completely get there without even more major changes to the registry).
fitzgen submitted PR review.
fitzgen created PR review comment:
I think it I think it makes sense to debug-assert the capacity condition, but also still
expect()here rather than?-propagate, just to sanity check that the actual behavior matches our expected/intended behavior.I wouldn't want to stop reserving space for this arena up front and rely on doubling behavior instead, as that could result in more reallocs and overallocation than is otherwise necessary.
fitzgen submitted PR review.
fitzgen created PR review comment:
Should this perhaps also debug assert that the capacity of self.type_to_rec_group is > ty.index()?
Wait we already have precisely this assertion:
fitzgen updated PR #12500.
fitzgen has enabled auto merge for PR #12500.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Oops not sure how I missed the assert being there... And yeah I'd agree that if there's an explicit reservation this should
.expect("..."), and the rationale to keep using explicit reservations vs natural growth makes sense to me too
fitzgen added PR #12500 Refactor the TypeRegistry to partially handle OOM to the merge queue.
fitzgen removed PR #12500 Refactor the TypeRegistry to partially handle OOM from the merge queue.
fitzgen merged PR #12500.
Last updated: Feb 24 2026 at 04:36 UTC