Stream: git-wasmtime

Topic: wasmtime / PR #13840 Shard pools for memories, tables, an...

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:35):

tschneidereit opened PR #13840 from tschneidereit:pooling-perf to bytecodealliance:main:

This changes the pooling allocator to use a shard per thread (capped at 16) of all relevant pools.
With this, lock contention in the pooling allocator is completely eliminated in local testing with a Rust Hello World WASIp2 HTTP proxy component.

performance of wasmtime serve -S cli hello.wasm goes from 135k RPS to about 168k RPS.

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:35):

tschneidereit added the label wasmtime:pooling-allocator on PR #13840.

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:35):

tschneidereit requested fitzgen for a review on PR #13840.

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:35):

tschneidereit requested wasmtime-core-reviewers for a review on PR #13840.

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:38):

tschneidereit added the label performance on PR #13840.

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:39):

tschneidereit updated PR #13840.

Wasmtime GitHub notifications bot (Jul 07 2026 at 15:58):

tschneidereit updated PR #13840.

Wasmtime GitHub notifications bot (Jul 07 2026 at 16:03):

:memo: fitzgen submitted PR review:

Great idea! A handful of comments below

Wasmtime GitHub notifications bot (Jul 07 2026 at 16:03):

:speech_balloon: fitzgen created PR review comment:

Can we define a ShardId newtype? And then have helpers on each pool type to go from ShardId to its sharded pool.

This would also let ShardId be represented as a u32, which should make some of the data structures here quite a bit more compact (eg the per_shard in free_many).

Wasmtime GitHub notifications bot (Jul 07 2026 at 16:03):

:speech_balloon: fitzgen created PR review comment:

This logic to distribute slots across shards is a little subtle, especially in the face of a pool whose capacity doesn't evenly divide by the shard size. Could we have a proptest or mutatis::check test that asserts that summing up each shards slots and max unused warm slots gives us the expected total? Might want to factor out some pure logic while doing this so that the available_parallelism call becomes a parameter and the property test isn't checking only the host's hardware configuration.

Wasmtime GitHub notifications bot (Jul 07 2026 at 16:03):

:speech_balloon: fitzgen created PR review comment:

This logic seems to be repeated a few times as well, could we wrap it up in a helper?

Wasmtime GitHub notifications bot (Jul 07 2026 at 16:03):

:speech_balloon: fitzgen created PR review comment:

Should flush_all_decommit_queues instead be something like flush_next_decommit_queue where it returns true as soon as any sharded queue's decommit returned a slot to the pool, rather than eagerly flushing all slots? This would avoid a number of lock acquisitions (at the cost of raising the chances that someone else steals the flushed slots before we get a chance to grab it). A good balance might be something like this:

let mut error = None;
assert!(self.decommit_queues.len() > 0);
for _ in 0..self.decommit_queues.len() {
    self.flush_next_decommit_queue() {
        match f() {
            Ok(x) => return Ok(x),
            Err(e) => {
                error = Some(e);
                continue;
            }
        }
    }
}
Err(error.unwrap())

Wasmtime GitHub notifications bot (Jul 07 2026 at 16:03):

:speech_balloon: fitzgen created PR review comment:

FWIW, I'm imagining something similar to our existing property test here: https://github.com/bytecodealliance/wasmtime/blob/6918520e213c1cce4c4f790907d2f9482056e3ed/crates/wasmtime/src/runtime/vm/instance/allocator/pooling/memory_pool.rs#L1015

Wasmtime GitHub notifications bot (Jul 07 2026 at 17:25):

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

Wasmtime GitHub notifications bot (Jul 07 2026 at 18:32):

tschneidereit updated PR #13840.

Wasmtime GitHub notifications bot (Jul 07 2026 at 18:34):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 07 2026 at 18:34):

:speech_balloon: tschneidereit created PR review comment:

Agreed yeah, and I wasn't too comfortable with the subtlety here either. I hadn't thought of using a proptest though, which I think is a great idea.

Wasmtime GitHub notifications bot (Jul 07 2026 at 18:42):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 07 2026 at 18:42):

:speech_balloon: tschneidereit created PR review comment:

Good call, yeah, and that's roughly what I implemented now. We could alternatively introduce something like free_many_and_alloc_one which would remove the risk of the slot being stolen. I'd like to at least do that in another PR though, given that the issue is preexisting, to the degree it's a real concern.

Wasmtime GitHub notifications bot (Jul 07 2026 at 18:42):

tschneidereit requested fitzgen for a review on PR #13840.

Wasmtime GitHub notifications bot (Jul 07 2026 at 19:27):

:thumbs_up: fitzgen submitted PR review:

LGTM, thanks!

Wasmtime GitHub notifications bot (Jul 07 2026 at 19:27):

fitzgen added PR #13840 Shard pools for memories, tables, and stacks to reduce/eliminate lock contention to the merge queue.

Wasmtime GitHub notifications bot (Jul 07 2026 at 19:52):

github-merge-queue[bot] removed PR #13840 Shard pools for memories, tables, and stacks to reduce/eliminate lock contention from the merge queue.

Wasmtime GitHub notifications bot (Jul 08 2026 at 11:36):

tschneidereit requested alexcrichton for a review on PR #13840.

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:memo: alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:speech_balloon: alexcrichton created PR review comment:

Could this method be deleted and tests updated to call deallocate_many?

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:speech_balloon: alexcrichton created PR review comment:

Did you see much benefit from using this? I'd naively expect this to be a bit too low level of a micro-optimization for us to see much benefit in the pooling allocator, but I'm also not entirely sure.

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:speech_balloon: alexcrichton created PR review comment:

FWIW if you drop the const this'll get evaluated at first-use meaning that the initialiation below with a fetch_add can be placed here directly and then it can be accessed as-is.

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:speech_balloon: alexcrichton created PR review comment:

Can this update the documentation of decommit_batch_size in config.rs to indicate that the queued-to-be-released can actually be up to 16 * (batch_size - 1) as opposed to batch_size - 1?

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:speech_balloon: alexcrichton created PR review comment:

I realize this is preexisting, but one possible improvement here would be to drop the queue lock before the closure f() is called to minimize the scope of the critical section here. Same for memory/table allocation below, too.

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:speech_balloon: alexcrichton created PR review comment:

I think this allocation is one reason CI is failing, notably this isn't handling OOM. We've got try_collect helpers though which can then be used to handle this

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:speech_balloon: alexcrichton created PR review comment:

Huh well ok given that preexisting this was .collect() maybe I'm totally off about what's causing the failure in fuzzing....

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:speech_balloon: alexcrichton created PR review comment:

This .collect() I think may also need to become try_collect to handle OOM

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:speech_balloon: alexcrichton created PR review comment:

I think this is failing on miri since this reads the local current directory, but feel free to put #[cfg_attr(miri, ignore)] on these proptest tests

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:speech_balloon: alexcrichton created PR review comment:

This I think is the same as decommit_shard_ids in the main pooling allocator, could there perhaps be a helper that returns an impl Iterator of shard ids starting with the current thread's home and wraps around?

Wasmtime GitHub notifications bot (Jul 08 2026 at 15:16):

:speech_balloon: alexcrichton created PR review comment:

A few thoughts on this: primarily could this be deduplicated with the above method? We've gotten ids/shards/etc wrong here in the past so it's subtle enough I'd prefer not to duplicate.

Otherwise though, could/should this use a SmallVec in the same manner as the index allocator? Looking at this again, too, this might be where the OOM tests are failing (OOM on the push here). We don't have an OOM-handling version of SmallVec, however, meaning that switching to SmallVec would perhaps paper over the OOM happening without actually addressing it, but that could be handled in a follow-up

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

tschneidereit updated PR #13840.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:speech_balloon: tschneidereit created PR review comment:

Done — removed, and the test now goes through deallocate_many.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:speech_balloon: tschneidereit created PR review comment:

Done — the thread-local now initializes straight from the fetch_add, no sentinel or Cell needed.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:speech_balloon: tschneidereit created PR review comment:

Done.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:speech_balloon: tschneidereit created PR review comment:

Done.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:speech_balloon: tschneidereit created PR review comment:

I didn't A/B this specifically, so I can't claim a measured win for it. The rationale: the shards are stored inline in the Box<[_]>, so without padding the tail of one shard's Inner/DecommitQueue (frequently-written fields) can end up on the same cache line as the next shard's mutex word. The cost is bounded (≤16 shards × ≤128 bytes of padding per pool), so I kept it as cheap insurance — but happy to drop it if you'd rather not carry it without a measurement.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:speech_balloon: tschneidereit created PR review comment:

That's actually already the case, though it's easy to miss: flush_decommit_queue takes the queue out of the mutex and drops the guard before doing the actual decommit, so neither the flush nor the retry of f() (nor the memory/table allocation retries below) runs while a queue lock is held. I extended the doc comment on with_flush_and_retry to make that explicit.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:13):

:speech_balloon: tschneidereit created PR review comment:

Done — this now uses try_collect, and the index-allocator constructors are fallible as well. FWIW, the abort in the OOM tests turned out to be pooling_allocator_fiber_stack_slot_leak_on_oom hitting the intermediate Vec that StackPool::deallocate_many / TablePool::deallocate_many built on the deallocation path; those now feed their iterator straight into free_many instead. The --cfg arc_try_new OOM suite aborted before these changes and passes with them (188/188 locally).

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:14):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:14):

:speech_balloon: tschneidereit created PR review comment:

Done — ShardLayout::new and the allocator constructors now return Result<_, OutOfMemory> and use try_collect.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:14):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:14):

:speech_balloon: tschneidereit created PR review comment:

The two fuzz-test failures in the queue run (smoke_test_memory_access and gc_ops_eventually_gcs) do look unrelated to this PR: they failed on the GC heap's minimum size exceeding the configured memory limit, i.e. an interaction with the state main was in during that merge-queue batch (#13841 was in flight at the time). Both tests pass locally on this branch merged with current main. The OOM-test abort was real, though — see the other thread.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:14):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:14):

:speech_balloon: tschneidereit created PR review comment:

Done — added a shard_ids_from_home helper in pooling.rs, used by both the decommit-queue paths and the index allocator's alloc probing.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:14):

:memo: tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 16:14):

:speech_balloon: tschneidereit created PR review comment:

Done on both counts: the subtle slot/stripe translation now lives in a shared return_slot helper used by both methods, and deallocate_many batches per-stripe frees in SmallVecs of the same shape as the index allocator's free_many. The OOM-on-spill caveat applies as you say and would be for a follow-up — the abort the OOM tests actually hit was the stack/table pools' intermediate Vecs, which are now gone entirely.

Wasmtime GitHub notifications bot (Jul 08 2026 at 17:11):

github-actions[bot] added the label wasmtime:config on PR #13840.

Wasmtime GitHub notifications bot (Jul 08 2026 at 17:21):

tschneidereit updated PR #13840.

Wasmtime GitHub notifications bot (Jul 08 2026 at 17:22):

tschneidereit commented on PR #13840:

Hold off on re-reviewing: I'm not actually done with this yet and hadn't intended to push for now :/

Wasmtime GitHub notifications bot (Jul 08 2026 at 17:29):

tschneidereit requested alexcrichton for a review on PR #13840.

Wasmtime GitHub notifications bot (Jul 08 2026 at 17:29):

tschneidereit commented on PR #13840:

@alexcrichton ok, I think this is ready again now

Wasmtime GitHub notifications bot (Jul 08 2026 at 17:34):

:speech_balloon: alexcrichton created PR review comment:

Ah ok, it make sense that it helps a bit but I feel like I keep reading various bits and pieces on the internet about how doing this is a bad idea. I think the bad idea part though specifically stems from taking up way too much space for frequently allocated things, which this doesn't fall under, so seems fine to leave

Wasmtime GitHub notifications bot (Jul 08 2026 at 17:34):

:memo: alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 17:35):

:memo: alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 08 2026 at 17:35):

:speech_balloon: alexcrichton created PR review comment:

Oh right I remember also missing that aspect the last time I looked at this too, thanks though!

Wasmtime GitHub notifications bot (Jul 08 2026 at 17:35):

alexcrichton added PR #13840 Shard pools for memories, tables, and stacks to reduce/eliminate lock contention to the merge queue.

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:01):

:check: alexcrichton merged PR #13840.

Wasmtime GitHub notifications bot (Jul 08 2026 at 18:01):

alexcrichton removed PR #13840 Shard pools for memories, tables, and stacks to reduce/eliminate lock contention from the merge queue.

Last updated: Jul 29 2026 at 05:03 UTC