Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Support running the pallet benchmarks analysis without running the benchmarks #12361

Conversation

koute
Copy link
Contributor

@koute koute commented Sep 27, 2022

This PR adds a new argument to the benchmarking CLI: --json-input <path.json>.

The idea is: you run the benchmarks once and generate raw results with --json-file, and then you can rerun the analysis and regenerate the weights as many times as you want, without actually rerunning the benchmarks.

This is in particular useful when working on the code responsible for the analysis since it makes it possible to do apples-to-apples comparisons while keeping the raw benchmark numbers constant.

Implementation notes

I've decided to just add an extra argument instead of a new command/subcommand since that seemed to be the least intrusive way to introduce this functionality, and there is already a precedent for it in the form of the --list argument; conceptually this might not be the cleanest, but should still be cheap to maintain and doesn't require rewriting half of the code.

I'm also open to suggestions in case someone has a better name for the command line argument. (:

@koute koute 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Sep 27, 2022
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont have suggestions about naming or anything like that, but yeah, like the feature

@koute
Copy link
Contributor Author

koute commented Sep 28, 2022

@ggwpez Applied your two suggestions; should be good to go!

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works perfectly, thanks!

@ggwpez
Copy link
Member

ggwpez commented Sep 28, 2022

/cmd queue -c fmt $ 1

@command-bot
Copy link

command-bot bot commented Sep 28, 2022

@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1892322 was started for your command "$PIPELINE_SCRIPTS_DIR/fmt.sh" 1. 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 /cmd cancel 84-bbfe4e88-014f-4979-be48-7f1563b34be2 to cancel this command or /cmd cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 28, 2022

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/fmt.sh" 1 has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1892322 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1892322/artifacts/download.

@koute
Copy link
Contributor Author

koute commented Sep 28, 2022

@ggwpez The zip file which the bot generated is empty; is this what you expected? (:

@ggwpez
Copy link
Member

ggwpez commented Sep 28, 2022

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 0ec4373 into paritytech:master Sep 28, 2022
@ggwpez
Copy link
Member

ggwpez commented Sep 28, 2022

The zip file which the bot generated is empty; is this what you expected? (:

That was just the formatting bot. No idea why it also produces empty zip files 🤷‍♂️ @koute

@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis/1026/2

@lisa-parity
Copy link
Contributor

polkadot-developers/substrate-docs#1621
Doc update:
| json-input <json-input-file> | Specifies the path to a JSON file with previously-generated benchmark results. This option enables you to reuse the benchmarks raw results generated with the --json-file to rerun the benchmark analysis and to regenerate the weights for a pallet without actually rerunning the benchmarks tests.

ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
…nchmarks (paritytech#12361)

* Support running the pallet benchmarks analysis without running the benchmarks

* Rename `override-results` to `json-input` and update the help comment

* ".git/.scripts/fmt.sh" 1

Co-authored-by: command-bot <>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants