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

Add gno std package function GasUsed #1998

Open
deelawn opened this issue Apr 29, 2024 · 9 comments
Open

Add gno std package function GasUsed #1998

deelawn opened this issue Apr 29, 2024 · 9 comments
Labels
🗺️good first issue🗺️ Ideal for newcomer contributors 📦 🤖 gnovm Issues or PRs gnovm related

Comments

@deelawn
Copy link
Contributor

deelawn commented Apr 29, 2024

Description

Let's add a function called GasUsed, or some name with the same sentiment, that returns the gas used until the current point in execution. It can obtain gas used from the gas meter recently added to the machine -- https://github.com/gnolang/gno/blob/master/gnovm/pkg/gnolang/machine.go#L61. To prevent abusing this function, we should define a reasonable amount of gas to be consumed when it is invoked. The documentation should also be updated as a part of the PR that adds this.

There are two potential use cases for this:

  1. This can be helpful for benchmarking gno code -- users trying to figure out which parts of their code is consuming large amounts of gas
  2. This can be used to measure performance of running a bit of code if someone would like to deploy a leetcode style realm that accepts interfaces with methods for solving a posed coding problem.
@deelawn deelawn added 🗺️good first issue🗺️ Ideal for newcomer contributors 📦 🤖 gnovm Issues or PRs gnovm related labels Apr 29, 2024
@notJoon
Copy link
Member

notJoon commented Apr 30, 2024

If it's possible to pull various data from the Machine, how about creating a package like runtime? I think it would be useful if we could directly check the memory usage by using Alloc field including gas usage, within the code.

type Allocator struct {
maxBytes int64
bytes int64
}

@zivkovicmilos
Copy link
Member

I like this idea a lot, we can take inspiration from what Solidity has:

https://docs.soliditylang.org/en/develop/units-and-global-variables.html#block-and-transaction-properties

@thinhnx-var
Copy link
Contributor

I am interested in this issue. I gonna try to implement this one 🙏

@thinhnx-var
Copy link
Contributor

thinhnx-var commented May 16, 2024

To prevent abusing this function, we should define a reasonable amount of gas to be consumed when it is invoked.

-> Should we defined the amount of gas in DefaultGasConfig in tm2 types package, or as const somewhere reasonable, and consumed it immediately when GasUsed() is invoked successfully? Can you give me some suggestions on this?
@deelawn @notJoon ...
Sorry if this tag is wrong targets 🙏

@thehowl
Copy link
Member

thehowl commented Jun 18, 2024

I was discussing with @moul on whether we want something like this.

We should always consider the implication of all the information we're giving to the developer. Like for the discussions we've had about whether to have PrevRealm/GetOrigCaller. If we're building a fully-deterministic system, any additional "context information" being passed in to the developer can be seen as an extra parameter to a "pure" function (in the mathematical sense; ie., a pure function with the same input will always yield the same output).

Let's take a look at the two use cases in the OP:

This can be helpful for benchmarking gno code -- users trying to figure out which parts of their code is consuming large amounts of gas
This can be used to measure performance of running a bit of code if someone would like to deploy a leetcode style realm that accepts interfaces with methods for solving a posed coding problem.

I'm unsure about the second one; but the first one sounds like it's more of a candidate for testing. It's something we partly have with the --print-runtime-metrics feature of gno test; and we should improve:

  • By having something like -cpuprofile to have a profiling system similar to pprof's, so (eventually) a developer can see a flamegraph of the gas consumption
  • By having a simple system to print, after each test, a count of the gas used.

Exposing gas on-chain, I think, also poses the risk of restricting us more if we want to change the way we count gas in the future.

So I'm thinking we should have this function either just as a testing standard library, or not at all.

@moul
Copy link
Member

moul commented Jun 18, 2024

I suggest we start by replacing execution time in unit tests with consumed gas, or keeping both side by side.

CleanShot 2024-06-18 at 12 00 12

This should be sufficient for most optimization needs, in my opinion.


Next, I believe the best targets are:

  • The debugger
  • A gno test -cover that could display information line by line

I would avoid adding std.GasUsed completely, and I've considered this option for a long time, but I've since changed my mind.
Instead, I propose adding std.TestGasUsed, limiting it to the testing context, only if the previous suggestions are not enough, but I guess they will be enough.

@thinhnx-var
Copy link
Contributor

In my view, displaying gasUsed in unit test and the gno test -cover / adding it to -print-runtime-metrics is clear enough for information and testing purpose. Since this gasUsed is limited in testing scope, we just need to consider about where and how to display it.
I am not so familiar with the debugger in gno, hence I dont have any idea.

Beside it, the -cpuprofile or profiling system is much more complex and gonna out of this discussion (as discussion in #2222 ).

@montoya83
Copy link

A display with the execution time+consumed gas isn't something that is going to tire anyone's eyes.
Keeping both -side by side- is much better, in my opinion.

@gfant
Copy link
Contributor

gfant commented Jul 24, 2024

This would be a great feature for Shiken @moul , since it would be useful to run the code onchain instead of off chain and avoid certain tasks for the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗺️good first issue🗺️ Ideal for newcomer contributors 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: Backlog
Development

No branches or pull requests

9 participants