wasmtime / issue #7255 Simplifying fuel APIs · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #7255 Simplifying fuel APIs

Wasmtime GitHub notifications bot (Oct 16 2023 at 16:44):

rockwotj opened issue #7255:

Feature & Benefit

Simplify the fuel related APIs.

Noted in https://github.com/bytecodealliance/wasmtime/pull/7240#issuecomment-1762549864, there are a number of fuel related APIs and they have grown organically - we should take a step back and see if there is a simpler/smaller scoped API that can serve the same purpose.

Proposal

Simplify fuel by removing "consumption" related tracking (with the right primitives embedders should be able to do this themselves).

Remove the following APIs:
pub fn fuel_consumed(&self) -> Option<u64>
pub fn fuel_remaining(&mut self) -> Option<u64>
pub fn add_fuel(&mut self, fuel: u64) -> Result<()>
pub fn consume_fuel(&mut self, fuel: u64) -> Result<u64>
pub fn out_of_fuel_trap(&mut self)
pub fn out_of_fuel_async_yield(
    &mut self,
    injection_count: u64,
    fuel_to_inject: u64
)
Introduce the following APIs:
// All APIs only return error if fuel is not enabled.

// get the current amount of fuel
pub fn get_fuel() -> Result<u64>
// set the current amount of fuel
pub fn set_fuel(u64) -> Result<()>
// in async mode, the engine should yield every N fuel consumed, or None to remove yielding
// Additionally Some(0) should return an error?
pub fn fuel_async_yield_interval(Option<u64>) -> Result<()>
Under the hood, this would work by storing the following state:
let fuel_reserves: u64 = 0;
let active_fuel: i64 = 0; // This is what the runtime increments as fuel is consumed
let yield_amount: Option<u64> = None;
Then get_fuel would be: return (-active_fuel).saturating_add(fuel_reserves)

fuel_async_yield_interval just assigns yield_amount then calls set_fuel(get_fuel())

set_fuel looks something like:
if yield_amount.is_none() {
  self.fuel_reserves = 0;
  self.active_fuel = -total_fuel;
  return;
}
let yield_amount = yield_amount.unwrap();
self.active_fuel = -cmp::min(fuel, yield_amount);
self.fuel_reserves = fuel + self.active_fuel;
Then out_of_gas does something like:
if self.fuel_reserves == 0 {
  return trap;
}
set_fuel(get_fuel());  // Easy mode to recalculate active_fuel and fuel_reserves
yield();
Alternatives

TODO(reader): Add your ideas :light_bulb: and comments :umm: to this issue :big_smile:

Wasmtime GitHub notifications bot (Oct 16 2023 at 18:20):

alexcrichton commented on issue #7255:

I like the idea of moving the idea of total consumption out of a Store and requiring embedders to track that if necessary. Additionally I like the simplicity here of get/set since that's at least what I'd imagine a counter like this to look like.

In that sense going over the existing APIs:

fuel_consumed - this would need to be replaced by embedders by keeping track of all fuel historically injected. This method would then be all_fuel_ever_injected - get_fuel().

fuel_remaining - this would become get_fuel()

add_fuel - this would be set_fuel(get_fuel() + n)

consume_fuel - this would be get_fuel(), some conditions, followed by maybe set_fuel.

That all sounds good to me and additionally handles reset_fuel added in https://github.com/bytecodealliance/wasmtime/pull/7240 by effectively renaming it to set_fuel.

Wasmtime GitHub notifications bot (Oct 16 2023 at 18:37):

rockwotj commented on issue #7255:

Cool - I'm happy to work on this later this week if there is no objections (I'm not sure who all needs/wants to sign off on this sort of thing), and if nobody wants to work on this earlier.

Wasmtime GitHub notifications bot (Oct 16 2023 at 22:46):

alexcrichton commented on issue #7255:

I'd recommend giving it a day or so for folks to chime in with feedback if they have some, otherwise feel free, and thanks again!

Wasmtime GitHub notifications bot (Oct 17 2023 at 00:56):

rockwotj commented on issue #7255:

One downside of this API is that we can only store up to i64 amount of fuel, so calculating consumption can be wrong:
set_fuel(u64::MAX).unwrap();
// run a function
let consumed = u64::MAX - get_fuel(); // WRONG! We actually started at i64::MAX
Can we change the fuel in vmcontext to be a u64 that goes to zero instead of a negative i64 that goes to 0?

Wasmtime GitHub notifications bot (Oct 17 2023 at 01:41):

rockwotj commented on issue #7255:

I guess that is an existing wart:
https://github.com/bytecodealliance/wasmtime/blob/9e4d44626a008f8a1b1b3b4a48f0d5693185844b/tests/all/fuel.rs#L198

I guess you always need to compare to get_fuel to implement tracking correctly?

Wasmtime GitHub notifications bot (Oct 17 2023 at 02:39):

rockwotj edited a comment on issue #7255:

One downside of this API is that we can only store up to i64 amount of fuel, so calculating consumption can be wrong:
set_fuel(u64::MAX).unwrap();
// run a function
let consumed = u64::MAX - get_fuel(); // WRONG! We actually started at i64::MAX

Wasmtime GitHub notifications bot (Oct 17 2023 at 14:05):

alexcrichton commented on issue #7255:

Yeah I think it'd be nice if we could fix that but I'm not sure how to do so myself, so in absence of that I think we sort of have to just stick with what we have today beahvior-wise.

Wasmtime GitHub notifications bot (Oct 23 2023 at 20:43):

alexcrichton closed issue #7255:

Feature & Benefit

Simplify the fuel related APIs.

Noted in https://github.com/bytecodealliance/wasmtime/pull/7240#issuecomment-1762549864, there are a number of fuel related APIs and they have grown organically - we should take a step back and see if there is a simpler/smaller scoped API that can serve the same purpose.

Proposal

Simplify fuel by removing "consumption" related tracking (with the right primitives embedders should be able to do this themselves).

Remove the following APIs:
pub fn fuel_consumed(&self) -> Option<u64>
pub fn fuel_remaining(&mut self) -> Option<u64>
pub fn add_fuel(&mut self, fuel: u64) -> Result<()>
pub fn consume_fuel(&mut self, fuel: u64) -> Result<u64>
pub fn out_of_fuel_trap(&mut self)
pub fn out_of_fuel_async_yield(
    &mut self,
    injection_count: u64,
    fuel_to_inject: u64
)
Introduce the following APIs:
// All APIs only return error if fuel is not enabled.

// get the current amount of fuel
pub fn get_fuel() -> Result<u64>
// set the current amount of fuel
pub fn set_fuel(u64) -> Result<()>
// in async mode, the engine should yield every N fuel consumed, or None to remove yielding
// Additionally Some(0) should return an error?
pub fn fuel_async_yield_interval(Option<u64>) -> Result<()>
Under the hood, this would work by storing the following state:
let fuel_reserves: u64 = 0;
let active_fuel: i64 = 0; // This is what the runtime increments as fuel is consumed
let yield_amount: Option<u64> = None;
Then get_fuel would be: return (-active_fuel).saturating_add(fuel_reserves)

fuel_async_yield_interval just assigns yield_amount then calls set_fuel(get_fuel())

set_fuel looks something like:
if yield_amount.is_none() {
  self.fuel_reserves = 0;
  self.active_fuel = -total_fuel;
  return;
}
let yield_amount = yield_amount.unwrap();
self.active_fuel = -cmp::min(fuel, yield_amount);
self.fuel_reserves = fuel + self.active_fuel;
Then out_of_gas does something like:
if self.fuel_reserves == 0 {
  return trap;
}
set_fuel(get_fuel());  // Easy mode to recalculate active_fuel and fuel_reserves
yield();
Alternatives

TODO(reader): Add your ideas :light_bulb: and comments :umm: to this issue :big_smile:

Last updated: Dec 23 2024 at 12:05 UTC