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

[ci] Add short benchmarks to the pipeline #5188

Merged
merged 14 commits into from
Mar 24, 2022
Merged

[ci] Add short benchmarks to the pipeline #5188

merged 14 commits into from
Mar 24, 2022

Conversation

alvicsam
Copy link
Contributor

@alvicsam alvicsam commented Mar 23, 2022 •
edited
Loading

The PR adds short benchmarks and removes check-runtime-benchmarks job as suggested in https://github.com/paritytech/ci_cd/issues/353

@alvicsam alvicsam 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 Mar 23, 2022
@alvicsam alvicsam requested a review from a team as a code owner March 23, 2022 10:42
@alvicsam alvicsam changed the title [Do not merge] Add short benchmarks to the pipeline [ci] Add short benchmarks to the pipeline Mar 23, 2022
coderobe
coderobe approved these changes Mar 23, 2022
View reviewed changes
scripts/run_short_benches_for_runtime.sh Outdated Show resolved Hide resolved
ggwpez
ggwpez approved these changes Mar 23, 2022
View reviewed changes
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.

Wow very cool! Nearly suspicious that they all succeed 😆

@brunopgalvao You can incorporate rococo here as well, once rococo benches work.

@alvicsam @coderobe
Update scripts/run_short_benches_for_runtime.sh
bd0a487 
Co-authored-by: Mara Robin B. <mara@broda.me>
@ggwpez
Copy link
Member

ggwpez commented Mar 23, 2022

I double checked and something is not quite right, but we can probably already merge this and fix that later:

The ump benchmark still does not work, but the script is not catching it since it seems to not execute it:

polkadot benchmark --execution wasm --wasm-execution compiled --chain polkadot-dev --pallet "runtime_parachains::ump" --extrinsic "*" --steps 1 --repeat 1

We can probably replace the script with just:

polkadot benchmark --execution wasm --wasm-execution compiled --chain ${runtime}-dev --pallet "*" --extrinsic "*" --steps 1 --repeat 1

@ggwpez
Copy link
Member

ggwpez commented Mar 23, 2022 •
edited
Loading

Yep now it detects the error, thanks for changing it. Maybe you can make it "allowed to fail" until we fix it?
PS: I will add an --exclude-pallet flag soon, so that we can then forbid it to fail.

Should be fixed now.

@ggwpez
Copy link
Member

ggwpez commented Mar 23, 2022

bot rebase

@paritytech-processbot
Copy link

Rebasing

@alvicsam
Copy link
Contributor Author

@ggwpez it seems that rebase didn't help. Should I make short-benchmarks allow_fail: true ?

@ggwpez
Copy link
Member

ggwpez commented Mar 24, 2022

it seems that rebase didn't help. Should I make short-benchmarks allow_fail: true ?

It helped, but now a different one fails 🤦 (pallet-bags-list). @alvicsam
Let's make it "allowed to fail" and the change it back once we either fixed them or added a --exclude-pallet.

@ggwpez ggwpez mentioned this pull request Mar 24, 2022
Merged
@alvicsam alvicsam merged commit 5bbaf2f into master Mar 24, 2022
@alvicsam alvicsam deleted the as-ci-short-bench branch March 24, 2022 12:52
ordian added a commit that referenced this pull request Mar 24, 2022
@ordian
Merge branch 'master' into ao-remove-old-migrations
b323434 
* master:
  Try to fix out of view statements (#5177)
  Companion for Substrate#11107 (#5197)
  paras: `include_pvf_check_statement` rt bench (#4938)
  [ci] Run short benchmarks only in PR pipelines (#5200)
  Companion for paritytech/substrate#10242 (#4862)
  [ci] Add short benchmarks to the pipeline (#5188)
  upgrade coarsetime to 0.1.22 to fix a potential panic (#5193)
  Update docs and enable DOT-over-XCM (#4809)
  enable disputes on all chains (#5182)
  companion for validator self-vote in bags (#5088)
  Extract MAX_FINALITY_LAG constant from relay_chain_selection (#5159)
@georgesdib georgesdib mentioned this pull request Mar 26, 2022
Merged
@alvicsam alvicsam mentioned this pull request Mar 31, 2022
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@coderobe coderobe coderobe approved these changes

@ggwpez ggwpez ggwpez approved these changes

@sergejparity sergejparity sergejparity approved these changes

@shawntabrizi shawntabrizi Awaiting requested review from shawntabrizi

Assignees
No one assigned
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alvicsam @ggwpez @coderobe @sergejparity