Stream: git-wasmtime

Topic: wasmtime / PR #13085 yield to executor when polling with ...

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:17):

dicej requested alexcrichton for a review on PR #13085.

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:17):

dicej opened PR #13085 from dicej:fix-13040 to bytecodealliance:main:

This addresses #13040 for wasmtime-wasi's WASIp2 implementation by calling tokio::task::yield_now in <Deadline as Pollable>::ready when self is a Deadline::Past. Previously, it did nothing, which had the effect of starving other pollables that rely on yielding control to mio in order to progress, e.g. when the guest polls with a zero timeout in a busy loop.

This also addresses that issue for wasmtime's thread.yield implementation by modifying the poll_until event loop in concurrent.rs to yield to the executor prior to executing any low-priority tasks. This works because thread.yield operates by queueing a low-priority task to resume the calling fiber just prior to suspending it.

Note that, because the wasmtime crate does not depend on Tokio, we can't directly call tokio::task::yield_now. Instead, we return a Future which, when polled for the first time, wakes the Context's Waker and returns Poll::Pending; then it returns Poll::Ready(()) for subsequent polls. That's enough to ensure mio has a chance to run when using tokio. Once we have a mechanism to allow the embedder to configure a custom yield function, we'll be able to use that instead.

Fixes #13040

<!--
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.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:17):

dicej requested wasmtime-wasi-reviewers for a review on PR #13085.

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:17):

dicej requested wasmtime-core-reviewers for a review on PR #13085.

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:30):

dicej updated PR #13085.

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:35):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:35):

alexcrichton created PR review comment:

I might recommend expanding this comment a bit to explain how what this is doing is a bit of a hack. The goal is that when a guest does a poll with a zero timeout in a loop we should eventually make our way back to Tokio, but if this function does nothing then that won't ever happen and we'll spin forever in the guest without actually yielding to Tokio. This is a best-attempt workaround to detect more-or-less this situation but is not really a full-fidelity fix per-se since there's other theoretical ways to busy loop too, but it's probably good enough for now nonetheless.

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:35):

alexcrichton created PR review comment:

Mind expanding on this a bit too like above? Here's less of a hack insofar as low priority here is "the guest explicitly yielded" and so we're faithfully respecting that request to go all the way back up to the host at least once. This could perhaps be expanded with docs along the lines of future optimizations too such as periodically yielding back rather than unconditionally yielding back or something like that, but we've got no need for that just yet.

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:35):

alexcrichton created PR review comment:

Could this be moved to a method on StoreOpaque? For example:

impl StoreOpaque {
    async fn yield_now(&mut self) {
        // ...
    }
}

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:35):

alexcrichton created PR review comment:

How come this is now taking a bool parameter? That seems like a subtle change from before which is unrelated to the issue-at-hand. Not that I think it's wrong, just curious.

Also, I find this method signature pretty confusing personally with bools-in-bools-out and subtle interactions between them. Another aspect is that I wouldn't exepct this to return true if nothing is actually pulled off the low priority queue either.

This function is only called once in this file, so could this be inlined directly? I think it'd be easier to follow all the bools if they're just local variables and logic to read.

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:35):

alexcrichton created PR review comment:

Also, could you double check we're not doing this sort of yield elsewhere in Wasmtime? I feel like we probably do this in at least one more location that can be deduplicated with this ideally.

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:43):

dicej created PR review comment:

Yeah, the bool parameter is partly an artifact of studying the code looking for problems while I debugged an issue that ended up being caused elsewhere. While I was doing that, I realized we were executing items from the low-priority queue rather than looping around to see if ConcurrentState::futures had more ready futures first, which felt like a priority inversion.

Anyway, you're right that it's not needed in this PR; I'll break it out, and I'll inline what remains.

Wasmtime GitHub notifications bot (Apr 13 2026 at 23:43):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Apr 14 2026 at 03:48):

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

Wasmtime GitHub notifications bot (Apr 14 2026 at 03:48):

github-actions[bot] added the label wasi on PR #13085.

Wasmtime GitHub notifications bot (Apr 14 2026 at 15:23):

dicej updated PR #13085.

Wasmtime GitHub notifications bot (Apr 14 2026 at 15:29):

dicej updated PR #13085.

Wasmtime GitHub notifications bot (Apr 14 2026 at 15:34):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Apr 14 2026 at 15:34):

dicej created PR review comment:

There was identical code in runtime/vm/async_yield.rs, which I've consolidated into the new StoreOpaque::yield_now function. There's one awkward case, though: StoreOpaque::do_gc uses unwrap_gc_store_mut to get exclusive access to the GcStore, which means we can't use the containing StoreOpaque during the call to GcStore::gc. For now, I'm just passing a freestanding function to GcStore::gc for it to use when it needs to yield. Once we have e.g. a Config::yield_fn field of type Option<fn() -> Box<dyn Future<Output = ()> + Send + Sync>>, we'll be able to pass in a closure which closes over that.

Wasmtime GitHub notifications bot (Apr 14 2026 at 15:39):

dicej updated PR #13085.

Wasmtime GitHub notifications bot (Apr 14 2026 at 15:49):

dicej updated PR #13085.

Wasmtime GitHub notifications bot (Apr 14 2026 at 15:57):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Apr 14 2026 at 15:57):

alexcrichton created PR review comment:

Accidental update?

Wasmtime GitHub notifications bot (Apr 14 2026 at 15:58):

dicej submitted PR review.

Wasmtime GitHub notifications bot (Apr 14 2026 at 15:58):

dicej created PR review comment:

oof, thanks for catching that; probably a rebase hiccup. I'll fix it.

Wasmtime GitHub notifications bot (Apr 14 2026 at 16:37):

dicej updated PR #13085.

Wasmtime GitHub notifications bot (Apr 14 2026 at 16:38):

dicej has enabled auto merge for PR #13085.

Wasmtime GitHub notifications bot (Apr 14 2026 at 16:49):

dicej added PR #13085 yield to executor when polling with zero-duration wait to the merge queue.

Wasmtime GitHub notifications bot (Apr 14 2026 at 17:43):

dicej merged PR #13085.

Wasmtime GitHub notifications bot (Apr 14 2026 at 17:43):

dicej removed PR #13085 yield to executor when polling with zero-duration wait from the merge queue.

Last updated: May 03 2026 at 22:13 UTC