Update benchmarking macros #3934

pgherveou · 2024-04-02T09:37:48Z

Current benchmarking macro returns a closure with the captured benchmarked code.
This can cause issues when the benchmarked code has complex lifetime requirements.

This PR updates the existing macro by injecting the recording parameter and invoking the start / stop method around the benchmarked block instead of returning a closure

One other added benefit is that you can write this kind of code now as well:

let v;
#[block]
{ v = func.call(); }
dbg!(v); // or assert something on v

Weights compare link

substrate/frame/benchmarking/src/utils.rs

ggwpez

Fine once we re-run the substrate benches. Thanks!

substrate/frame/support/procedural/src/benchmark.rs

substrate/frame/benchmarking/src/utils.rs

pgherveou · 2024-04-02T11:33:44Z

substrate/frame/staking/src/benchmarking.rs

@@ -1077,15 +1077,13 @@ mod tests {
 				(frame_benchmarking::BenchmarkParameter::v, v),
 				(frame_benchmarking::BenchmarkParameter::n, n),
 			];
-			let closure_to_benchmark =


can this test be removed, it should be executed when running the benchmark tests anyway ?

Yea looks a bit weird.

ah nvm it's marked as #[extra] so I guess this is to ensure the test runs everytime..

pgherveou · 2024-04-02T11:40:02Z

bot help

command-bot · 2024-04-02T11:40:06Z

Here's a link to docs

pgherveou · 2024-04-02T11:42:32Z

bot bench-all substrate

command-bot · 2024-04-02T11:42:36Z

@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5730507 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench-all/bench-all.sh" --target_dir=substrate. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-e2f49155-d59b-42ba-95a0-e568537d0eff to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2024-04-02T12:28:13Z

@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench-all/bench-all.sh" --target_dir=substrate has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5730507 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5730507/artifacts/download.

pgherveou · 2024-04-02T16:09:38Z

bot bench-all substrate

command-bot · 2024-04-02T16:09:42Z

@pgherveou "$PIPELINE_SCRIPTS_DIR/commands/bench-all/bench-all.sh" --target_dir=substrate (https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5736938) was cancelled in #3934 (comment)

pgherveou · 2024-04-02T17:02:47Z

bot cancel 2-0444d85c-3a2b-4901-aa42-5e499a58ce60

command-bot · 2024-04-02T17:03:03Z

@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench-all/bench-all.sh" --target_dir=substrate has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5736938 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5736938/artifacts/download.

pgherveou · 2024-04-02T17:03:07Z

bot bench-all substrate

command-bot · 2024-04-02T17:03:12Z

@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5737796 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench-all/bench-all.sh" --target_dir=substrate. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 3-41cdd745-28bd-4ad4-adae-a90a48ba89f3 to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2024-04-08T13:03:34Z

@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5822874 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench-all/bench-all.sh" --target_dir=substrate. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-b771a94b-ab21-4d4e-a3fa-ff99ad54f1ee to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2024-04-08T13:06:49Z

@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench-all/bench-all.sh" --target_dir=substrate has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5822874 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5822874/artifacts/download.

ggwpez · 2024-04-08T21:31:37Z

bot bench-all substrate

command-bot · 2024-04-08T21:31:42Z

@ggwpez https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5831222 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench-all/bench-all.sh" --target_dir=substrate. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-b2242482-db25-434e-b302-4acdf5610c21 to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2024-04-09T03:41:57Z

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/commands/bench-all/bench-all.sh" --target_dir=substrate has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5831222 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5831222/artifacts/download.

pgherveou · 2024-04-09T05:55:53Z

here are the result compared to the weights freshly ran on master
https://weights.tasty.limo/compare?unit=weight&ignore_errors=true&threshold=10&method=asymptotic&repo=polkadot-sdk&old=pg/fix-weights&new=pg/bench_update&path_pattern=substrate/frame/**/src/weights.rs,polkadot/runtime/*/src/weights/**/*.rs,polkadot/bridges/modules/*/src/weights.rs,cumulus/**/weights/*.rs,cumulus/**/weights/xcm/*.rs,cumulus/**/src/weights.rs

pgherveou · 2024-04-10T05:09:11Z

@bkontur @acatangiu can I get a +1 from one of view for the bridge review lock?

serban300 · 2024-04-10T06:27:07Z

@pgherveou could you just revert the change in the bridges folder please ? I think it's just noise. It looks like it wouldn't make a difference. But it would create conflicts for our migration PR: #4024

pgherveou · 2024-04-10T06:36:08Z

@serban300 I just fixed the constant because the tests were failing, how is that calculated?

serban300 · 2024-04-10T06:43:30Z

@serban300 I just fixed the constant because the tests were failing, how is that calculated?

Oh ok. I'm not sure how it's calculated. But we can keep it if it's needed for the test to pass.

serban300

The bridges change looks ok.

Update benchmarking

a3ddd5e

pgherveou marked this pull request as ready for review April 2, 2024 09:40

pgherveou requested a review from a team as a code owner April 2, 2024 09:40

pgherveou changed the title ~~Update benchmarking~~ Update benchmarking macros Apr 2, 2024

ggwpez reviewed Apr 2, 2024

View reviewed changes

substrate/frame/benchmarking/src/utils.rs Outdated Show resolved Hide resolved

ggwpez reviewed Apr 2, 2024

View reviewed changes

substrate/frame/support/procedural/src/benchmark.rs Show resolved Hide resolved

substrate/frame/benchmarking/src/utils.rs Outdated Show resolved Hide resolved

Fix tests

db22ac0

paritytech-review-bot bot requested a review from a team April 2, 2024 11:02

pgherveou added 2 commits April 2, 2024 13:26

PR doc

d3a6ccb

fix staking benchmark

03ba18e

pgherveou commented Apr 2, 2024

View reviewed changes

fix prdoc

31e7f4c

pgherveou added the T12-benchmarks This PR/Issue is related to benchmarking and weights. label Apr 2, 2024

pgherveou added 2 commits April 2, 2024 16:27

Add on_before_start closure

7b4d0c8

Fix fix

a560470

pgherveou force-pushed the pg/bench_update branch from 37ff3c0 to a560470 Compare April 2, 2024 16:05

fix comment

c92091b

pgherveou added 2 commits April 2, 2024 18:43

fix

d701524

fix

f311681

Merge branch 'master' into pg/bench_update

59c44a5

".git/.scripts/commands/bench-all/bench-all.sh" --target_dir=substrate

e3bffd2

pgherveou mentioned this pull request Apr 9, 2024

Weights tests #4026

Closed

pgherveou enabled auto-merge April 9, 2024 08:42

athei approved these changes Apr 9, 2024

View reviewed changes

athei and others added 2 commits April 9, 2024 11:57

Merge branch 'master' into pg/bench_update

6823690

Update constant

3c87518

paritytech-review-bot bot requested a review from a team April 9, 2024 11:14

pgherveou added 2 commits April 9, 2024 18:07

Merge branch 'master' into pg/bench_update

253c2cf

Merge branch 'master' into pg/bench_update

3575b22

serban300 approved these changes Apr 10, 2024

View reviewed changes

pgherveou added this pull request to the merge queue Apr 10, 2024

Merged via the queue into master with commit d38f6e6 Apr 10, 2024
133 of 136 checks passed

pgherveou deleted the pg/bench_update branch April 10, 2024 07:20

clangenb mentioned this pull request May 30, 2024

polkadot v1.12.0 open-web3-stack/open-runtime-module-library#1003

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update benchmarking macros #3934

Update benchmarking macros #3934

pgherveou commented Apr 2, 2024 •

edited

Loading

ggwpez left a comment

pgherveou Apr 2, 2024

ggwpez Apr 5, 2024

pgherveou Apr 8, 2024

pgherveou commented Apr 2, 2024

command-bot bot commented Apr 2, 2024

pgherveou commented Apr 2, 2024

command-bot bot commented Apr 2, 2024 •

edited

Loading

command-bot bot commented Apr 2, 2024

pgherveou commented Apr 2, 2024

command-bot bot commented Apr 2, 2024 •

edited

Loading

pgherveou commented Apr 2, 2024

command-bot bot commented Apr 2, 2024

pgherveou commented Apr 2, 2024

command-bot bot commented Apr 2, 2024 •

edited

Loading

command-bot bot commented Apr 8, 2024 •

edited

Loading

command-bot bot commented Apr 8, 2024

ggwpez commented Apr 8, 2024

command-bot bot commented Apr 8, 2024 •

edited

Loading

command-bot bot commented Apr 9, 2024

pgherveou commented Apr 9, 2024

pgherveou commented Apr 10, 2024

serban300 commented Apr 10, 2024 •

edited

Loading

pgherveou commented Apr 10, 2024

serban300 commented Apr 10, 2024

serban300 left a comment

Update benchmarking macros #3934

Update benchmarking macros #3934

Conversation

pgherveou commented Apr 2, 2024 • edited Loading

ggwpez left a comment

Choose a reason for hiding this comment

pgherveou Apr 2, 2024

Choose a reason for hiding this comment

ggwpez Apr 5, 2024

Choose a reason for hiding this comment

pgherveou Apr 8, 2024

Choose a reason for hiding this comment

pgherveou commented Apr 2, 2024

command-bot bot commented Apr 2, 2024

pgherveou commented Apr 2, 2024

command-bot bot commented Apr 2, 2024 • edited Loading

command-bot bot commented Apr 2, 2024

pgherveou commented Apr 2, 2024

command-bot bot commented Apr 2, 2024 • edited Loading

pgherveou commented Apr 2, 2024

command-bot bot commented Apr 2, 2024

pgherveou commented Apr 2, 2024

command-bot bot commented Apr 2, 2024 • edited Loading

command-bot bot commented Apr 8, 2024 • edited Loading

command-bot bot commented Apr 8, 2024

ggwpez commented Apr 8, 2024

command-bot bot commented Apr 8, 2024 • edited Loading

command-bot bot commented Apr 9, 2024

pgherveou commented Apr 9, 2024

pgherveou commented Apr 10, 2024

serban300 commented Apr 10, 2024 • edited Loading

pgherveou commented Apr 10, 2024

serban300 commented Apr 10, 2024

serban300 left a comment

Choose a reason for hiding this comment

pgherveou commented Apr 2, 2024 •

edited

Loading

command-bot bot commented Apr 2, 2024 •

edited

Loading

command-bot bot commented Apr 2, 2024 •

edited

Loading

command-bot bot commented Apr 2, 2024 •

edited

Loading

command-bot bot commented Apr 8, 2024 •

edited

Loading

command-bot bot commented Apr 8, 2024 •

edited

Loading

serban300 commented Apr 10, 2024 •

edited

Loading