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(benchmarking): tune benchmarking for non-return syscalls #3319

Merged
merged 9 commits into from
Sep 26, 2023

Conversation

tltsutltsu
Copy link
Contributor

@tltsutltsu tltsutltsu commented Sep 21, 2023

Resolves: #2938

There are a couple of benchmarks preceded with this comment:

// We cannot call `gr_wait` multiple times. Therefore our weight determination is not 
// as precise as with other APIs. 

Lets call them "one-time benchmarks"

This PR resolves this problem by splitting the benchmarking process of pallet_gear into two runs. The first run is made with default parameters (--steps=20, --repeat=50) for all extrinsics, excluding one-time ones. The second run is made only for one-time extrinsics, with --steps=2, --repeat=1000 params. This allows to evaluate one-time benchmarks more thoroughly.

Here's how it's done, technically:

  • run_all_benchmarks.sh obtains a list of all extrinsics.
  • run_all_benchmarks.sh runs all benchmarks, excluding one-time ones. The output is written into the pallet_gear.rs file.
  • run_all_benchmarks.sh runs only one-time benchmarks. The output is written into the pallet_gear_onetime.rs file.
  • merge-outputs.sh moves WeightInfo trait definitions and weights from the pallet_gear_onetime.rs file to main pallet_gear.rs file.

No changes needed in CI, and everything, including weights unpacking script, is are already working fine.
Successful benchmark action run / Weights diff table

The set of these one-time benchmarks can be changed anytime without need of modifying the logic in the script. Additionally , merge-outputs.sh also allows to merge more than one file by specifying multiple files.

@gear-tech/dev

@tltsutltsu tltsutltsu added A1-inprogress Issue is in progress or PR draft is not ready to be reviewed D6-gas Change gas charging logic or weights labels Sep 21, 2023
@tltsutltsu tltsutltsu self-assigned this Sep 21, 2023
@tltsutltsu tltsutltsu changed the title Tune benchmarking for non-return syscalls feat(benchmarking): tune benchmarking for non-return syscalls Sep 21, 2023
@tltsutltsu tltsutltsu marked this pull request as ready for review September 22, 2023 10:28
@tltsutltsu tltsutltsu added A0-pleasereview PR is ready to be reviewed by the team and removed A1-inprogress Issue is in progress or PR draft is not ready to be reviewed labels Sep 22, 2023
@ukint-vs
Copy link
Member

Could you post a table here with the weight changes?

@tltsutltsu
Copy link
Contributor Author

@ukint-vs

Could you post a table here with the weight changes?

Sure!

name master tuned diff
gr_reply_commit 20.8 µs 16.7 µs +24.61%
gr_reply_commit_wgas 16.0 µs 23.4 µs -31.69%
gr_reservation_reply 11.1 µs 10.2 µs +8.71%
gr_reservation_reply_commit 5.3 µs 10.3 µs -48.34%
gr_reply 16.8 µs 24.5 µs -31.20%
gr_reply_wgas 20.5 µs 21.7 µs -5.37%
gr_reply_input 21.9 µs 20.2 µs +8.30%
gr_reply_input_wgas 0 ps 30.9 µs N/A
gr_exit 23.7 µs 181.1 µs -86.92%
gr_leave 12.2 µs 172.9 µs -92.95%
gr_wait 13.2 µs 149.0 µs -91.17%
gr_wait_for 14.0 µs 185.5 µs -92.46%
gr_wait_up_to 12.2 µs 167.1 µs -92.68%

These weights were measured with 1000 repeats.

Here is a full table with other weights

@breathx breathx merged commit 14ecf12 into master Sep 26, 2023
11 checks passed
@breathx breathx deleted the tltsutltsu/tune-benchmarking-for-non-return-syscalls branch September 26, 2023 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-pleasereview PR is ready to be reviewed by the team D6-gas Change gas charging logic or weights
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tune benchmarking for non-return sys-calls
4 participants