-
Notifications
You must be signed in to change notification settings - Fork 379
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: gnovm benchmarking tool #2241
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2241 +/- ##
==========================================
- Coverage 61.10% 60.46% -0.64%
==========================================
Files 564 572 +8
Lines 75355 79491 +4136
==========================================
+ Hits 46045 48066 +2021
- Misses 25945 27822 +1877
- Partials 3365 3603 +238
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Perhaps we could move the transient size
variable so that is defined inside the benchmarking package as an exported variable. Then it can be set and read strictly within benchmarking constant conditional blocks and avoid any unnecessary allocations when benchmarking is not enabled.
What do you think?
benchmarking/Makefile
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be reorganised to:
gnovm/cmd/gnobench
+gnovm/pkg/gasbench
ORcontribs/gnobecnh
?
I don't want to add another top-level directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gnovm/pkg/opsbench
Waiting for updates as per discussions with Ray |
…ive benchmark anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preliminary review.
gnovm/pkg/benchops/bench.go
Outdated
|
||
// StopMeasurement ends the current measurement and resumes the previous one | ||
// if one exists. It accepts the number of bytes that were read/written to/from | ||
// the store. This value is zero if the operation is not a read or write. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comment seems incorrect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch!
if bm.OpsEnabled { | ||
bm.PauseOpCode() | ||
defer bm.ResumeOpCode() | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
metric storage here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was purposely left out. GetType(id) is part of a recursive call flow through the realm's fillType() function. To provide an accurate benchmarking measurement, we moved away from the stack-based measurement approach, which involved pushing and pausing a timer on the stack when the runtime entered a function call. The trade-off is that we can no longer measure the runtime of functions involved in recursive calls. For now, we can assume and assign the same gas value to GetType as we do for SetType.
panic("Can not stop a stopped timer") | ||
} | ||
measure.opAccumDur[code] += time.Since(measure.opStartTime[code]) | ||
measure.opStartTime[code] = measure.timeZero // stop the timer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unset isOpCodeStarted
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I will fix it.
Hey @piux2, can you rebase this with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pr provides:
-
Performance measurement capabilities for opcodes (count, execution time) and persistent storage (count, size). This part is similar to performance counter registers in processors. It would better be called
perf
(the VM capability) instead ofbenchops
(a particular usage of this capability, and not the only possible one, i.e profiling, gas measurement). See for example linux or freebsd -
An executable called
gnobench
, to perform a benchmark on a specific scenario (a sample forum board in gno). This executable should not be located under gnovm, but incontribs/benchmark
, with
its dependencies:- the readme, makefile, etc in
contrib/benchmark
- the executable itself in
contrib/benchmark/cmd/gnobench
- benchmarked gno files in
contrib/benchmark/gno
- the vm part still in
gnovm/pkg/perf
- the readme, makefile, etc in
Regarding the build tags to enable or not the counting, I'm not a big fan of that. It brings a lot of hassle, and it's partly useless because counting is necessary in production anyway. The performance gain vs using a variable instead of a constant (requiring a different executable) to enable the feature in opcode procressing was shown negligible when we introduced the debugger, it should be the same here. A former colleague of mine used to say: "Premature optimisation is the root of all evil" :-)
In more details, the measurement of opcodes consists of counting and measuring execution time. Counting is super cheap (a counter increment operation), and could be always performed. Measuring execution time is probably a bit more costly and it makes sense to enable it only for benchmark scenarios. In production, we could have a static pre-computed profile of mean exec time per opcode, for gas estimation. But again, no sufficient reason to use build tags.
Regarding the storage of performance counters, they should be stored per virtual machine, instead of a global variable measure
, which is generally a bad practice, bringing a lot of unstated constraints and risks. I understand that it is already the case for other parts of the VM, and that it facilitates the coding, but this is something that we should eventually fix. It can be done later, I don't want to block the PR for that.
I'm a bot that assists the Gno Core team in maintaining this repository. My role is to ensure that contributors understand and follow our guidelines, helping to streamline the development process. The following requirements must be fulfilled before a pull request can be merged. These requirements are defined in this configuration file. Automated Checks🟢 Maintainers must be able to edit this pull request Manual ChecksNo manual checks match this pull request. Debug
|
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the descriptionWe build this tool mainly for the following issues
#1826
#1828
#1281
#1973
We could also use it in the following cases.
#1973
#2222
gnobench
benchmarks the time consumed for each VM CPU OpCode and persistent access to the store, including marshalling and unmarshalling of realm objects.Design consideration
Minimum Overhead and Footprint
Accuracy
It is built on top of @deelawn's design and framework with @jaekwon's input.
#2073
Usage
Simple mode
The benchmark only involves the GnoVM and the persistent store. It benchmarks the bare minimum components, and the results are isolated from other components. We use standardize gno contract to perform the benchmarking.
This mode is the best for benchmarking each major release and/or changes in GnoVM.
Production mode
It benchmarks the node in the production environment with minimum overhead.
We can only benchmark with standardize the contract but also capture the live usage in production environment.
It gives us a complete picture of the node perform.
go build -tags "benchmarkingstorage benchmarkingops" gno.land/cmd/gnoland
Run the node in the production environment. It will dump benchmark data to a benchmark.bin file.
call the realm contracts at
gno.land/r/x/benchmark/opcodes
andgno.land/r/x/benchmark/storage
Stop the server after the benchmarking session is complete.
Run the following command to convert the binary dump:
gnobench -bin path_to_benchmark_bin
Results ( Examples )
The benchmarking results are stored in two files: