Refactors election provider support benchmarks outside its own crate by gpestana · Pull Request #13431 · paritytech/substrate

gpestana · 2023-02-21T17:20:15Z

The bench bot is having issues with running the elections-provider-support benchmarks and updating the weights.rs because the current elections provider support benchmark code is a separate crate.

It seems that bench bot --output default is the same as the crate where the benchmarks are implemented. Since the --output cannot be re-written in the bench bot CI, the output folder path is not the correct one. (Note: on top of this, the bench bot EXCLUDE_PALLET list needs to be updated, which is done in this PR as well).

This PR removes the need for an separate benchmarking crate for elections-provider-support. This way, the dir where the CI bot pushes the weights match that of the expected by the CI bot.

Expected behaviour:
The following command works locally as expected (ie. generated weights.rs are added under /frame/election-provider-support/src/weights.rs)

cargo run --release --features runtime-benchmarks benchmark pallet \
  --execution wasm \
  --wasm-execution compiled \
  --dev --pallet "pallet-elections-provider-support" \
  --extrinsic "*" \
  --steps 2 \
  --repeat 1 \
  --output frame/elections-provider-support/src/weights.rs \
  --template .maintain/frame-weight-template.hbs

frame/election-provider-support/src/weights.rs has been updated with the bench bot.

gpestana · 2023-02-21T22:32:38Z

bot bench $ pallet dev pallet_election_provider_support

command-bot · 2023-02-21T22:32:43Z

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

Comment bot cancel 30-ae4ed2fb-d94c-4df5-a6d4-908d0c3291e8 to cancel this command or bot cancel to cancel all commands in this pull request.

gpestana · 2023-02-21T22:37:50Z

bot bench $ pallet dev pallet_election_provider_support

command-bot · 2023-02-21T22:37:58Z

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

Comment bot cancel 31-0f599949-b2a9-4bf1-9511-03143703f3d4 to cancel this command or bot cancel to cancel all commands in this pull request.

paritytech-cicd-pr · 2023-02-21T22:45:54Z

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2425790

command-bot · 2023-02-21T22:50:25Z

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_election_provider_support has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2425776 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2425776/artifacts/download.

command-bot · 2023-02-21T22:54:32Z

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_election_provider_support has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2425827 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2425827/artifacts/download.

gpestana · 2023-02-21T22:55:06Z

bot bench $ pallet dev pallet_election_provider_support

command-bot · 2023-02-21T22:55:11Z

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

Comment bot cancel 33-e0649b53-7888-4bcd-afe8-8db3b052dd3d to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2023-02-21T23:23:34Z

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_election_provider_support has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2425944 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2425944/artifacts/download.

gpestana · 2023-02-22T09:18:42Z

bot bench $ pallet dev frame_election_provider_support

command-bot · 2023-02-22T09:18:47Z

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

Comment bot cancel 38-89b286cc-823d-49ce-873e-8630c6563c1e to cancel this command or bot cancel to cancel all commands in this pull request.

…vider_support

command-bot · 2023-02-22T09:55:07Z

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev frame_election_provider_support has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2427398 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2427398/artifacts/download.

ruseinov

This looks like a good simplification to me.
The only question I have is what was the reason to have this as a separate crate in the first place. Looking at the change set I cannot find any, but maybe the original author could chip in.

kianenigma

weight seem to have been submitted now 💯

…ench

* Abstracts elections-phragmen pallet to use NposSolver * Update frame/elections-phragmen/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/elections-phragmen/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * changes the name of the pallet; adds changelog * update changelog * Adds weight testing * Adds log macro_rules * renames elections-phragment dir to elections * weights rename * fixes typo in cargo toml * pre/post solve weight scafolding * refactor do_post_election * refactors into pre and post election solve for independent benchmarking * deconstructs PreElectionResults struct * updates benchmarking pre and post election solve; mock weights * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * addresses PR comments * adds pre_solve and post_sove weights * Adds comments on election pallet id param name change * ".git/.scripts/bench-bot.sh" pallet dev pallet_elections * Finishes pre-post solve weights * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Addresses PR comments: no panic in on_init path; nits * Fixes node build * Implements approval voting to use as a `NposSolver` (#13367) * Implements the approval voting methods in sp_npos_elections * fmt * remove unecessary file * comment clarification * re-run weights * fix typo * updates MaxVoters in tests for integrity_tests to pass * Refactors election provider support benchmarks outside its own crate (#13431) * Refactors election provider support benchmarks outside its own crate --------- Co-authored-by: command-bot <> --------- Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: parity-processbot <> Co-authored-by: Ross Bulat <ross@parity.io>

* Abstracts elections-phragmen pallet to use NposSolver * Update frame/elections-phragmen/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/elections-phragmen/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * changes the name of the pallet; adds changelog * update changelog * Adds weight testing * Adds log macro_rules * renames elections-phragment dir to elections * weights rename * fixes typo in cargo toml * pre/post solve weight scafolding * refactor do_post_election * refactors into pre and post election solve for independent benchmarking * deconstructs PreElectionResults struct * updates benchmarking pre and post election solve; mock weights * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * addresses PR comments * adds pre_solve and post_sove weights * Adds comments on election pallet id param name change * ".git/.scripts/bench-bot.sh" pallet dev pallet_elections * Finishes pre-post solve weights * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Addresses PR comments: no panic in on_init path; nits * Fixes node build * Implements approval voting to use as a `NposSolver` (paritytech#13367) * Implements the approval voting methods in sp_npos_elections * fmt * remove unecessary file * comment clarification * re-run weights * fix typo * updates MaxVoters in tests for integrity_tests to pass * Refactors election provider support benchmarks outside its own crate (paritytech#13431) * Refactors election provider support benchmarks outside its own crate --------- Co-authored-by: command-bot <> --------- Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: parity-processbot <> Co-authored-by: Ross Bulat <ross@parity.io>

gpestana added 4 commits February 21, 2023 18:19

Refactors election provider support benchmarks outside its own crate

3a9969f

Add approvals voting code

0b4ee7c

fixes node benches

32b24b7

fix benches

2ccf5c0

gpestana marked this pull request as draft February 21, 2023 22:33

fixes

1dc77a2

fixes

a027233

".git/.scripts/commands/bench/bench.sh" pallet dev frame_election_pro…

17233c0

…vider_support

gpestana self-assigned this Feb 22, 2023

gpestana marked this pull request as ready for review February 22, 2023 09:58

gpestana added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Feb 22, 2023

gpestana requested review from Ank4n, kianenigma and rossbulat February 22, 2023 09:58

gpestana added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label Feb 22, 2023

ruseinov self-requested a review February 22, 2023 11:56

ruseinov approved these changes Feb 22, 2023

View reviewed changes

kianenigma approved these changes Feb 22, 2023

View reviewed changes

Merge branch 'gpestana8250_npossolver' into gpestana8250_npossolver_b…

fc00052

…ench

gpestana merged commit 70ea87a into gpestana8250_npossolver Feb 22, 2023

gpestana deleted the gpestana8250_npossolver_bench branch February 22, 2023 13:10

gpestana mentioned this pull request Feb 22, 2023

Abstracts elections-phragmen pallet to use NposSolver #12588

Merged

6 tasks

gpestana mentioned this pull request Feb 23, 2023

Abstracts elections-phragmen pallet to use NposSolver (v2) #13453

Open

Conversation

gpestana commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gpestana commented Feb 21, 2023

Uh oh!

command-bot bot commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gpestana commented Feb 21, 2023

Uh oh!

command-bot bot commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paritytech-cicd-pr commented Feb 21, 2023

Uh oh!

command-bot bot commented Feb 21, 2023

Uh oh!

command-bot bot commented Feb 21, 2023

Uh oh!

gpestana commented Feb 21, 2023

Uh oh!

command-bot bot commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

command-bot bot commented Feb 21, 2023

Uh oh!

gpestana commented Feb 22, 2023

Uh oh!

command-bot bot commented Feb 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

command-bot bot commented Feb 22, 2023

Uh oh!

ruseinov left a comment

Choose a reason for hiding this comment

Uh oh!

kianenigma left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gpestana commented Feb 21, 2023 •

edited

Loading

command-bot bot commented Feb 21, 2023 •

edited

Loading

command-bot bot commented Feb 21, 2023 •

edited

Loading

command-bot bot commented Feb 21, 2023 •

edited

Loading

command-bot bot commented Feb 22, 2023 •

edited

Loading