Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(state-viewer): simple gas counter extraction #7733

Closed
wants to merge 6 commits into from

Conversation

jakmeier
Copy link
Contributor

Reverse-engineer the charged gas counters for an executed function call
in the state viewer. This is useful to have in the state viewer because the
RPC request for tx-status only gives the total gas values per cost.

A next step building on top would be another state viewer command that
takes a set of parameter changes and a range of blocks. It could then
find receipts that are successful with the old config but would run out of
gas with the new parameters. This is useful to evaluate any sort of gas
increase.

So far, only the counters for WASM execution / host function calls
are supported. Adding action counters would be much more effort
and probably it makes more sense to include more details in the
gas profile first.

@jakmeier jakmeier requested a review from a team as a code owner September 29, 2022 21:19
@jakmeier jakmeier requested review from mina86 and matklad and removed request for mina86 September 29, 2022 21:19
@jakmeier
Copy link
Contributor Author

@matklad I really want some debugging tooling like proposed in this PR. For now just extracting gas counters for one receipt. Next step would be to apply a parameter diff to it and recompute the cost. And do that with entire block ranges,

We needed such a tool in the past, we need it now, and we will likely need it again in the future. So far we usually ended up replaying entire ranges with manual one-shot code changes and parsing scripts, such as the one from Edvard. I don't want to write yet another such script, so I'm trying to make it more generally useful. :)

But it is a bit awkward to implement. Our gas profiles and parameters are still not connected directly. To make it work here, I added a mapping Parameter -> Cost. And then a ProfileData -> Map<Parameter,u64> function that reverse-calculates the gas counters using the first mapping. Sadly, it only works for all WASM costs, as the mapping for actions is not one-to-one. Action costs could also be important, although I can live without them for now. But in any case, it just feels messy the way I am doing it.

As reminder: Today we use Parameter / ParamterTable to construct RuntimeConfig in JSON format, and then we manually lookup values inside that in many places. Cost / ProfileData is for the gas profile and many action cost parameters are conflated into single costs. Knowing just the profile values, it is not possible to assess parameter changes, so some reverse calculation is necessary.

The estimator has its own enum Cost. Coding a relationship from those to Parameter and/or Cost for profiles is also still open.

Long-term, I think expanding the Cost for gas profiles such that they store per-byte and base costs separately would make sense. That would also make a complete Parameter -> Cost mapping more viable to implement. I just don't think this is very high priority to do right now, so I don't really want to spend too much time on this right now.

But I wonder if you think it makes sense to have some incomplete tool like this merged in. Or maybe you see some better way and I am going in the complete wrong direction here.

@matklad
Copy link
Contributor

matklad commented Sep 30, 2022

Hm, so the code here reminds me of #5719. I think here we implement exactly the same "gas -> counts" logic we used there.

I guess my super-high level comment is that we have too much infrastructure here already:

  • Costs in estimator
  • Costs in profile
  • parameters
  • with_ext_cost infra

So, before we add something here, I wonder if we can remove anything?

In particular, this with_ext_cost thing seems like half of what we are doing here.

@jakmeier
Copy link
Contributor Author

I approached the use case to find out the effect of gas changes a bit differently in another PR: #8483

How much gas exactly was spent per parameter is now also available directly in profiles, thanks to #8148.

Closing this for now.

@jakmeier jakmeier closed this Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants