Support reset_fuel in store APIs

[rockwotj]

Add a simpler interface to reset the amount of fuel in a store.

Fixes: #5109

A continuation of #5220, which has stalled. To make it more obvious that consumed_fuel is reset, I've changed the name to this method to be reset_fuel.

[github-actions]

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasmtime:api", "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:api, wasmtime:c-api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[fitzgen]

Thanks! Just one documentation nitpick below to be addressed and then we can merge this.

[alexcrichton]

One thing that's perhaps worth noting is that we have quite a litany of fuel-related methods and we seem to keep adding more over time. This isn't necessarily a problem, nor do I think this is a blocker for this PR, but I think it indicates that we haven't done a great job of picking the right abstraction related to fuel. Or perhaps they're all independently motivated but given the ad-hoc nature of the additions I suspect we can do better. The fuel methods we have right now are:

• fuel_consumed
• fuel_remaining
• add_fuel
• consume_fuel

To be clear this is mostly food-for-thought. I do not wish to block this PR, and if @fitzgen approves we should merge.

[rockwotj]

I'm happy to come up with a better API before this PR (or make breaking changes, not sure what the policy is on that).

TBH we use fuel today and I didn't realize that consumed fuel was tracked independently of remaining fuel. Personally I probably would have pushed fuel consumed to embedders and make them track it based on how much fuel was remaining after each call into wasmtime. But I know taking away functionality is hard, but most people I've seen use fuel in the issue tracker are just attempting to prevent the engine from hogging too much CPU or getting stuck in a loop. Also I think if that functionality didn't exist you could potentially only have a set_fuel method and get_fuel method and let embedders calculate the delta. But instead there are different methods to remove fuel vs adding fuel which complicates the API vs just 'tell us how much fuel exists'.

Not sure if that makes since, but that would be the simplest primitive and API, as well as still supporting all the functionality (even if some would be pushed to embedders like tracking consumption).

Additionally, I personally find out_of_fuel_async_yield a complicated API in terms of the number of times to inject fuel. IMO the simpler API is either a method to auto reset fuel after yield to a specific value (and to trap you change out of gas behavior in the store then reset fuel to 0 and then resume the VM [as another note I'm not sure if it's valid to drop the future and still use the VM for yielding support]) or there is a yield every N fuel and you trap after fuel runs out (there is no adding fuel automatically from the API although it might do that under the hood). Basically how can we get that method to only a single number input.

If its valid to make breaking changes, I'd be happy to take a stab at a simpler fuel API based on the above in a followup PR.

[rockwotj]

RE breaking changes I found https://github.com/bytecodealliance/wasmtime/blob/main/docs/stability-release.md which my reading of that means we would need an RFC to simplify these APIs? Not sure if this would be major or not but what I'm proposing seems to be.

[rockwotj]

The more I think about this the more I like the appeal of four fuel APIs in Store, I believe you could still achieve everything there is today:

get_fuel() -> Result<u64>
set_fuel(u64) -> Result<()>
fuel_async_yield_interval(u64) -> Result<()>
fuel_async_yield_interval_clear() -> Result<()>

Basically you can set and get the amount of fuel and then to enable async support and yielding we say every N fuel consumed we will yield. This also means you could get rid of the out of gas behavior APIs. Consolidating something like 7 (8 including this PR) methods into 4

Internally there is a current active fuel, and a reserve "tank" of fuel for restoring after a yield. The get and set operate on the combined amount of active fuel and fuel in the tank, then the current enum for out of gas behavior just becomes an optional amount of fuel to take from the tank after a yield. If the yield interval is cleared we just take all the fuel from the tank and add it to the active amount.

That feels way simpler to explain and implement. With a tighter API surface to boot. TBH i don't fully understand how fuel_adj works in the current implementation.

[alexcrichton]

I want to be sure to reiterate that this isn't something you're expected to take on @rockwotj, although if you're willing we won't stop you of course! Additionally breaking changes are ok to Wasmtime and happen relatively frequently. We only require major breaking changes to go through RFCs, although that hasn't happened in quite some time. This change to fuel would not constitue "major" and could be done through consensus of various stakeholders.

I'd have to set aside some time to think about this personally, so if you're interested, would you be up for filing an issue with your idea and we could discuss on there? If not that's totally ok and I can file an issue in the future as well.

[rockwotj]

Haha I get nerdsniped easily 😄

I created an issue here with my strawman proposal: #7255

[fitzgen]

LGTM! We can land this in the meantime as the details in that other issue are hashed out.