Wasmtime GitHub notifications bot (Aug 26 2026 at 15:00): Wasmtime GitHub notifications bot (Aug 26 2026 at 15:00):adamrk requested cfallin for a review on PR #14213.
Wasmtime GitHub notifications bot (Aug 26 2026 at 15:00):adamrk requested wasmtime-compiler-reviewers for a review on PR #14213.
Wasmtime GitHub notifications bot (Aug 26 2026 at 15:00):adamrk requested wasmtime-core-reviewers for a review on PR #14213.
Wasmtime GitHub notifications bot (Aug 26 2026 at 15:01):adamrk opened PR #14213 from adamrk:defer-bulk-op-fuel-consumption to bytecodealliance:main:
For bulk operations and memory/table grows, defer consuming fuel until the operation has succeed.
This fixes https://github.com/bytecodealliance/wasmtime/issues/14161.
For small statically known sizes, we immediately consume fuel, but for larger or dynamic sizes we insert a fuel/epoch check before the operation and then only consume the variable fuel amount after.
Wasmtime GitHub notifications bot (Aug 26 2026 at 15:06):adamrk updated PR #14213.
Wasmtime GitHub notifications bot (Aug 26 2026 at 15:09):adamrk updated PR #14213.
Wasmtime GitHub notifications bot (Aug 26 2026 at 15:12)::memo: adamrk submitted PR review.
Wasmtime GitHub notifications bot (Aug 26 2026 at 15:12)::speech_balloon: adamrk created PR review comment:
Since we're now consuming the fuel after the fuel check, a bunch of these tests need an additional fuel check inserted at the end to pass. The empty loop block does that.
Wasmtime GitHub notifications bot (Aug 26 2026 at 15:12)::speech_balloon: adamrk created PR review comment:
I'm not sure what to do with these saturating fuel tests. Now that we only consume the fuel on a successful operation we shouldn't be able to actually hit the saturation.
Wasmtime GitHub notifications bot (Aug 26 2026 at 18:06)::memo: alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Aug 26 2026 at 18:06)::speech_balloon: alexcrichton created PR review comment:
Could the fuel check move to after the operation instead of before the operation to handle this? That way while the fuel running out would be retroactively discovered it'd basically be the same thing and wouldn't need extra updates
Wasmtime GitHub notifications bot (Aug 26 2026 at 18:37)::memo: adamrk submitted PR review.
Wasmtime GitHub notifications bot (Aug 26 2026 at 18:37)::speech_balloon: adamrk created PR review comment:
Yeah we can do that. I think it'll require changing some other tests which assert that we don't do a bulk operation if fuel is at 0 just before the op, but that seems fine to me.
Wasmtime GitHub notifications bot (Aug 26 2026 at 22:24)::memo: alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Aug 26 2026 at 22:24)::speech_balloon: alexcrichton created PR review comment:
Yeah that seems ok to me, a lot of the fuel tests are about precise behavior and it's ok to tweak them if the behavior is slightly adjusted as well
Wasmtime GitHub notifications bot (Aug 28 2026 at 20:54):adamrk updated PR #14213.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:03)::memo: adamrk submitted PR review.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:03)::speech_balloon: adamrk created PR review comment:
One thing that's a little weird now is that we use fuel to decide if the operation is small and then that determines if we skip the epoch check after the operation. So if a user set the fuel
cost_per_unitto 0 and ran with epochs then we wouldn't run the epoch check after a large bulk operation. Not sure if that needs to be fixed - alternatives would be to always do the epoch check, or have distinct conditions for a "small operation" when epochs are enabled.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:04):adamrk updated PR #14213.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:04):adamrk requested alexcrichton for a review on PR #14213.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:10):alexcrichton unassigned cfallin from PR #14213 fuel: Defer fuel consumption for bulk operations.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:19):adamrk updated PR #14213.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:30)::thumbs_up: alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:30)::speech_balloon: alexcrichton created PR review comment:
Could the various permutations of this test all get added to
fuel.wastinstead? Naively it seems like that'd work, but I might be missing something too
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:30)::speech_balloon: alexcrichton created PR review comment:
It seems useful to test a number of operators here such as the growth ones and
memory.fill, and in retrospect I think all of this test could be added tofuel.wastperhaps? Trapping behavior can't be added there but it seems reasonable to me to skip testing the trapping behavior since the semantics there are sort of inherited from general translation infrastructure which may already be tested too.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:30)::speech_balloon: alexcrichton created PR review comment:
I was thinking this debug assert was suspicious because we should able to get here with just epochs, and then I saw CI and looks like it additionally discovered that.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:30)::speech_balloon: alexcrichton created PR review comment:
That to me feels like an obscure enough edge case that it's ok. If an embedder says these operations have 0 cost then skipping the epoch check almost seems faithful to that since if it's free why bother checking epochs in more places.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:30)::speech_balloon: alexcrichton created PR review comment:
Is there any particular reason to single out
memory.fillhere vs other operations? If this is specifically a regression test for one of the original failures I think it's ok to skip this test since it'll be covered by fuzzing
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:30):adamrk updated PR #14213.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:32)::memo: adamrk submitted PR review.
Wasmtime GitHub notifications bot (Aug 28 2026 at 21:32)::speech_balloon: adamrk created PR review comment:
Yeah sorry, just fixed it.
Wasmtime GitHub notifications bot (Aug 31 2026 at 19:33)::memo: adamrk submitted PR review.
Wasmtime GitHub notifications bot (Aug 31 2026 at 19:33)::speech_balloon: adamrk created PR review comment:
Yeah it was just to explicitly cover the case found in fuzzing (in reduced form). I'll remove it.
Wasmtime GitHub notifications bot (Sep 01 2026 at 13:08):adamrk updated PR #14213.
Wasmtime GitHub notifications bot (Sep 01 2026 at 13:10)::memo: adamrk submitted PR review.
Wasmtime GitHub notifications bot (Sep 01 2026 at 13:10)::speech_balloon: adamrk created PR review comment:
Sounds good, I removed the test and added to fuel.wast so each bulk memory operation has 3 tests there for the small const, large const, and dynamic cases.
Wasmtime GitHub notifications bot (Sep 01 2026 at 13:11)::memo: adamrk submitted PR review.
Wasmtime GitHub notifications bot (Sep 01 2026 at 13:11)::speech_balloon: adamrk created PR review comment:
Moved these to fuel.wast as well.
table.growandmemory.grownow each have 6 tests infuel.wastfor the 3 size cases and success/failure.
Wasmtime GitHub notifications bot (Sep 01 2026 at 13:15):adamrk requested alexcrichton for a review on PR #14213.
Wasmtime GitHub notifications bot (Sep 01 2026 at 13:34)::thumbs_up: alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 01 2026 at 13:34):alexcrichton added PR #14213 fuel: Defer fuel consumption for bulk operations to the merge queue.
Wasmtime GitHub notifications bot (Sep 01 2026 at 14:00):github-merge-queue[bot] removed PR #14213 fuel: Defer fuel consumption for bulk operations from the merge queue.
Wasmtime GitHub notifications bot (Sep 01 2026 at 15:35):adamrk commented on PR #14213:
Looks like the merge queue failure was a real failure from an oob test case now trapping with a different error and the test should be changed. I'm not sure why that wasn't picked up on the PR CI run?
Wasmtime GitHub notifications bot (Sep 01 2026 at 15:43):alexcrichton commented on PR #14213:
Oh we've got some more information here but CI on PRs is a subset of the CI run in the merge queue (it'd be a bit too expensive to run everything on every PR)
Wasmtime GitHub notifications bot (Sep 01 2026 at 15:44):adamrk commented on PR #14213:
I see it was a failure on i686. Makes sense that we don't run that on every PR.
Wasmtime GitHub notifications bot (Sep 01 2026 at 18:09):adamrk updated PR #14213.
Wasmtime GitHub notifications bot (Sep 01 2026 at 18:14):adamrk commented on PR #14213:
So the i686 failure was actually an unrelated bug that was just uncovered by this change:
When handlingtable.growwe would trap with an OOB error if the delta couldn't fit in a nativeusize, meaning on 32-bit host a 64-bit table grow by more thanu32::MAXwould trap when it should actually return -1.It's just a one line fix so I included it here along with an explicit test for this behavior.
Wasmtime GitHub notifications bot (Sep 01 2026 at 18:15):adamrk requested alexcrichton for a review on PR #14213.
Wasmtime GitHub notifications bot (Sep 01 2026 at 18:35)::thumbs_up: alexcrichton submitted PR review:
Oh nice catch!
Wasmtime GitHub notifications bot (Sep 01 2026 at 18:35):alexcrichton added PR #14213 fuel: Defer fuel consumption for bulk operations to the merge queue.
Wasmtime GitHub notifications bot (Sep 01 2026 at 19:00)::check: alexcrichton merged PR #14213.
Wasmtime GitHub notifications bot (Sep 01 2026 at 19:00):alexcrichton removed PR #14213 fuel: Defer fuel consumption for bulk operations from the merge queue.
Last updated: Sep 20 2026 at 19:05 UTC