Gas metering #2414
Gas metering #2414
Conversation
leoyvens
force-pushed
the
zac/gas
branch
2 times, most recently
from
61ff7c2 to
30720a8
Compare
|
@That3Percent I did a few improvements in the last 5 commits. This is ready for review. |
leoyvens
force-pushed
the
zac/gas
branch
3 times, most recently
from
3a0fad4 to
4f57452
Compare
runtime/wasm/src/gas/combinators.rs
Outdated
|
|
||
| #[inline] | ||
| fn const_gas_size_of() -> Option<Gas> { | ||
| None |
There was a problem hiding this comment.
This could have an impl which should lead to better performance due to const propagation
| Self::const_gas_size_of().expect("GasSizeOf unimplemented") | ||
| } | ||
| /// Some when every member of the type has the same gas size. | ||
| #[inline(always)] |
There was a problem hiding this comment.
Note: Because this has the "implement one or the other" requirement, it would seem natural to split GasSizeOf and ConstGasSizeOf and add a blanket impl for GasSizeOf for T where T: ConstGasSizeOf. The reason this is not this way is because it would require specialization for tuples and that is not a stable feature.
graph/src/runtime/gas/mod.rs
Outdated
| pub const ZERO: Gas = Gas(0); | ||
| } | ||
|
|
||
| impl From<u64> for Gas { |
There was a problem hiding this comment.
I regret this impl and it's inverse (below). Seems convenient but ambiguous.
There was a problem hiding this comment.
I've removed them
| @@ -376,6 +392,7 @@ impl WasmInstance { | |||
| let host_metrics = host_metrics.cheap_clone(); | |||
| let timeout_stopwatch = timeout_stopwatch.cheap_clone(); | |||
| let ctx = ctx.cheap_clone(); | |||
| let gas = gas.cheap_clone(); | |||
There was a problem hiding this comment.
Why is the gas cloned here? Is this safe?
There was a problem hiding this comment.
GasCounter holds an Arc internally so this is a reference being cloned.
runtime/wasm/src/module/mod.rs
Outdated
| @@ -428,6 +442,7 @@ impl WasmInstance { | |||
|
|
|||
| for module in modules { | |||
| let func_shared_ctx = Rc::downgrade(&shared_ctx); | |||
| let gas = gas.cheap_clone(); | |||
There was a problem hiding this comment.
Again the gas clones. Seems suspicious to me that gas might get lost possibly within a single mapping function
| bytes: &Vec<u8>, | ||
| gas: &GasCounter, | ||
| ) -> Result<serde_json::Value, DeterministicHostError> { | ||
| gas.consume_host_fn(gas::DEFAULT_GAS_OP.with_args(gas::complexity::Size, &bytes))?; |
There was a problem hiding this comment.
I expect this should have a reasonably expensive OP, since parsing JSON is slow and hard. Comparing to h160 parsing (below) - the latter is practically free.
There was a problem hiding this comment.
True that the computational cost is very different, but I think the default throughput assumption of 10MB/s is pessimistic enough to be a lower bound to the performance of json parsing.
|
@leoyvens is this the gas costing work - are we planning to merge this in? |
|
Yes, we plan to merge this, I just need to find a moment to rebase and deploy it. |
leoyvens
force-pushed
the
zac/gas
branch
3 times, most recently
from
249a6f4 to
38a38f2
Compare
This will probably be necessary for multiblockchain.
|
Could be realated to #2112 (comment) How is possible to set a experimental feature flag when running the graph? |
|
The env var is |
|
Hi, @leoyvens! Thanks for your work on the awesome project! Sorry if this is a silly question, but... Do I understand correctly that the "gas metering" measures how much resources a Subgraph mapper is using? So when there's a infinite loop in a mapper then it's going to quickly run into these limitations, right? If so, I'm wondering whether the gas metering is done per block or per event? |
|
Hi @kasparkallas, you understand correctly. The limit is per trigger (such as an EVM event). |
This PR picks up on @That3Percent's work on gas metering for handlers, see #2038.
I tried this out on the Livepeer and the Uniswap subgraphs. Our target gas per nanosecond is 10, I got numbers between 1 and 1000 for that metric, which varies a lot for different calls to the same handler. The underestimation cases are the most concerning since if we underestimate too much that defeats the purpose of gas metering, but 1 gas per nanosecond is within one order of magnitude from the target so I consider it to be an acceptable underestimate. 1000 gas nanosecond per second is two orders of magnitude of error, but even those handlers didn't use more than a few billion gas, so they could be much more expensive and not hit the limit so the overestimate cases also seem acceptable. This still needs to be tried out in the integration to see if any important subgraphs hit the limit.
On the indexing performance impact, I measured each call to the
gashost fn to take less than 100ns, which is fast, but that export is called tens of thousands of times on typical handlers, so it can add up to a millisecond per handler, which is a measurable but not major impact, and I don't think there is much of a way around it.