wip on metering host fns #100
Conversation
| self.wasmer_metering_remaining_points | ||
| .as_ref() | ||
| .ok_or(wasm_error!(WasmErrorInner::Memory))? | ||
| .set(store_mut, 0i32.into()) |
There was a problem hiding this comment.
How do these points get returned to the pool? It seems like not keeping part of the capacity requested to be claimed would mean that returning capacity on completion could get out of sync?
There was a problem hiding this comment.
It looks like that's not the idea. You allocate a certain amount of capacity and when it's gone it's gone until something calls set to top it back up?
I still wonder if accurately tracking the value to account for overrun and making the 'set' an 'increment by' makes sense?
There was a problem hiding this comment.
@ThetaSinner well changing the middleware from decreasing points to increasing would mean rewriting the middleware from the ground up
we're using the default setup from wasmer, and what i'm doing here is exposing it so that it can be accessed by host functions as well as the default wasm op counter
There was a problem hiding this comment.
I don't mean dropping the increasing part, the increasing part is fine. I'm looking at set_remaining_points and wondering if that should be a paired up increment rather than a set?
If this is just exposing something wasmer already does then fine
There was a problem hiding this comment.
@ThetaSinner it's a direct port of https://github.com/wasmerio/wasmer/blob/master/lib/middlewares/src/metering.rs#L335 but not requiring the instance itself to be in the environment, all we need is the global
| .try_into() | ||
| .unwrap(); | ||
|
|
||
| assert!(before_decrease - after_decrease == dec_by); |
There was a problem hiding this comment.
Minor, optional:
assert_eq! here?
Then separate assertions on the next line so it's clear what failed if something goes wrong?
Co-authored-by: Jost Schulte <[email protected]>
No description provided.