Stream: git-wasmtime

Topic: wasmtime / PR #12565 Refactor `String` usage in `wasmtime...

view this post on Zulip Wasmtime GitHub notifications bot (Feb 10 2026 at 22:18):

fitzgen opened PR #12565 from fitzgen:use-oom-handling-string-in-env-module to bytecodealliance:main:

Do not hold regular Strings; instead use our own OOM-handling
wasmtime_core::alloc::String or, even better, an interned Atom from the
StringPool.

Also avoid IndexMap, as it doesn't handle OOMs.

Depends on

view this post on Zulip Wasmtime GitHub notifications bot (Feb 10 2026 at 22:18):

fitzgen requested wasmtime-compiler-reviewers for a review on PR #12565.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 10 2026 at 22:18):

fitzgen requested cfallin for a review on PR #12565.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 10 2026 at 22:18):

fitzgen requested wasmtime-core-reviewers for a review on PR #12565.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 10 2026 at 22:24):

alexcrichton commented on PR #12565:

Knee-jerk reaction before you have too many more PRs that depend on this -- IndexMap is a relatively fundamental dependency especially for components, so I'm not sure we'll be able to escape making an OOM friendly version. Or at least I recall my impression being that we have quite a lot of IndexMaps throughout compilation/etc. Is the usage pretty localized to just one or two locations though?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 10 2026 at 22:33):

fitzgen commented on PR #12565:

Knee-jerk reaction before you have too many more PRs that depend on this -- IndexMap is a relatively fundamental dependency especially for components, so I'm not sure we'll be able to escape making an OOM friendly version. Or at least I recall my impression being that we have quite a lot of IndexMaps throughout compilation/etc. Is the usage pretty localized to just one or two locations though?

IndexMap is either index_map::IndexMap or a BTreeMap depending on wasmparser configuration:

https://github.com/bytecodealliance/wasmtime/blob/755979dd81eb79ff746fb4e63d4570513216e731/crates/environ/src/prelude.rs#L30

std/alloc does not provide a way to fallibly reserve/insert/allocate for BTreeMaps.

And index_map::IndexMap does not provide a try_reserve method either. Additionally, index_map::IndexMap's Deserialize implementation requires that the key be Clone, which our OOM-handling Strings are most definitely not.

So it seems like continuing to use IndexMap is going to be an uphill battle.

We could make our own IndexMap that is a full implementation, rather than simply a newtype wrapper over either of those existing implementations. But that also seems pretty annoying and like more of a maintenance burden than is ideal... But maybe not, given that what I did with exports here is basically inlining/hard-coding one specific instance of that?

Thoughts?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 00:48):

alexcrichton commented on PR #12565:

One possible fix might be to use indexmap::IndexMap (directly from the crate, not wasmparser). That should support no_std and it looks like it's got a try_reserve to work for us as well. For Deserialize wouldn't we be writing custom impls as opposed to using any built-in ones too?

I guess what I'm saying is that my leaning is that we should have an indexmap::IndexMap wrapper the same way we have a Vec wrapper and then replace the usage of wasmparser::IndexMap with that fallible IndexMap.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 03:39):

github-actions[bot] added the label cranelift on PR #12565.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 03:39):

github-actions[bot] added the label wasmtime:api on PR #12565.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 19:50):

fitzgen updated PR #12565.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 19:51):

fitzgen commented on PR #12565:

I guess what I'm saying is that my leaning is that we should have an indexmap::IndexMap wrapper the same way we have a Vec wrapper and then replace the usage of wasmparser::IndexMap with that fallible IndexMap.

Okay, that has been done and I've pushed a new commit that switches this back to using IndexMap instead of multiple {Primary,Secondary}Maps.

Also this depends on these other PRs now:

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 21:12):

fitzgen updated PR #12565.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 21:12):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 21:12):

alexcrichton created PR review comment:

This I think is no longer needed?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 21:13):

fitzgen updated PR #12565.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 21:13):

fitzgen has enabled auto merge for PR #12565.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 21:27):

fitzgen added PR #12565 Refactor String usage in wasmtime_environ::Module to the merge queue

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 22:15):

fitzgen merged PR #12565.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 22:15):

fitzgen removed PR #12565 Refactor String usage in wasmtime_environ::Module from the merge queue

Last updated: Feb 24 2026 at 04:36 UTC