Skip to content

Added short-benchmarks for cumulus#1183

Merged
bkontur merged 14 commits intomasterfrom
bko-short-benchmarks-for-cumulus
Sep 1, 2023
Merged

Added short-benchmarks for cumulus#1183
bkontur merged 14 commits intomasterfrom
bko-short-benchmarks-for-cumulus

Conversation

@bkontur
Copy link
Copy Markdown
Contributor

@bkontur bkontur commented Aug 25, 2023

This PR adds new jobs for short-benchmarks for system parachain runtimes in cumulus subdirectory which was added to the Cumulus repo here.

@bkontur bkontur added A3-backport Pull request is already reviewed well in another branch. T9-cumulus This PR/Issue is related to cumulus. T12-benchmarks This PR/Issue is related to benchmarking and weights. T14-system_parachains This PR/Issue is related to system parachains. labels Aug 25, 2023
@paritytech-ci paritytech-ci requested review from a team August 25, 2023 22:45
bkontur
bkontur commented Aug 25, 2023
View reviewed changes
Comment thread .gitlab/pipeline/short-benchmarks.yml

# run short-benchmarks for system parachain runtimes from cumulus

.short-benchmark-cumulus: &short-bench-cumulus
Copy link
Copy Markdown
Contributor Author

@bkontur bkontur Aug 25, 2023

Choose a reason for hiding this comment

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

.short-benchmark-cumulus and .short-benchmark-polkadot could be possibly merged to .short-benchmark-template with some variables parametrization, do we want to do it?

bkontur
bkontur commented Aug 25, 2023
View reviewed changes
Comment thread .gitlab/pipeline/build.yml
- cargo build --profile release --locked --features=runtime-benchmarks --bin polkadot
- mkdir -p artifacts
- target/release/polkadot --version
- cp ./target/release/polkadot ./artifacts/
Copy link
Copy Markdown
Contributor Author

@bkontur bkontur Aug 25, 2023

Choose a reason for hiding this comment

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

There is also mv ./target/testnet/polkadot ./artifacts/. in the step build-linux-stable, so this step overrides that, right?
Possibly we could do something like: cp ./target/release/polkadot ./artifacts/short-benchmarks/.

Also the same for polkadot-parachain binary build-linux-stable-cumulus does mv ./target/release/polkadot-parachain ./artifacts/. but also build-short-benchmark-cumulus does `cp ./target/release/polkadot-parachain ./artifacts/.

I am not missing anything, there is a possible consequence that later jobs could use binary with --features runtime-benchmarks.

@paritytech-ci paritytech-ci requested a review from a team August 29, 2023 12:07
@gilescope
Copy link
Copy Markdown
Contributor

gilescope commented Aug 31, 2023

Given that it's short, could we do it all in one job rather than giving the CI another 10+ jobs to handle? In fact could we do the relay chain ones in the same job (also rococo on the relay jobs is currently missed out fyi)?

@bkontur
Copy link
Copy Markdown
Contributor Author

bkontur commented Sep 1, 2023

Given that it's short, could we do it all in one job rather than giving the CI another 10+ jobs to handle? In fact could we do the relay chain ones in the same job (also rococo on the relay jobs is currently missed out fyi)?

I think separate jobs per runtime is much more clear or transparent.
They all reuse the same prebuild binaries, so the execution itself is "fast".
They can run in "parallel" (depends on free vm runners I guess).
You can see exactly which runtime has problem, so it is easier for identification.
You can even see, which runtimes are configured and which not.

To Roocco, I dont know, it was missing also in old polkadot repo.

@bkontur bkontur enabled auto-merge (squash) September 1, 2023 10:05
alvicsam
alvicsam requested changes Sep 1, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@alvicsam alvicsam left a comment

Choose a reason for hiding this comment

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

@gilescope is correct. Lets use another runners for these benchmarks

Comment thread .gitlab/pipeline/short-benchmarks.yml
@paritytech-ci paritytech-ci requested a review from a team September 1, 2023 10:19
bkontur and others added 2 commits September 1, 2023 12:34
@bkontur @alvicsam
Update .gitlab/pipeline/short-benchmarks.yml
14db007 
Co-authored-by: Alexander Samusev <41779041+alvicsam@users.noreply.github.com>
@bkontur
Added tag/benchmarks also to the short-benchmark-polkadot
e0c662b
@bkontur bkontur requested a review from alvicsam September 1, 2023 10:38
@bkontur bkontur merged commit 15cb0af into master Sep 1, 2023
@bkontur bkontur deleted the bko-short-benchmarks-for-cumulus branch September 1, 2023 11:27
ordian added a commit that referenced this pull request Sep 1, 2023
@ordian
Merge branch 'master' into s0me0ne/candidate-session-index
40e3da9 
* master: (25 commits)
  fix typos (#1339)
  Use bandersnatch-vrfs with locked dependencies ref (#1342)
  Bump bs58 from 0.4.0 to 0.5.0 (#1293)
  Contracts: `seal0::balance` should return the free balance (#1254)
  Logs: add extra debug log for negative rep changes (#1205)
  Added short-benchmarks for cumulus (#1183)
  [xcm-emulator] Improve hygiene and clean up (#1301)
  Bump the known_good_semver group with 1 update (#1347)
  Renames API (#1186)
  Rename `polkadot-parachain` to `polkadot-parachain-primitives` (#1334)
  Add README to project root (#1253)
  Add environmental variable to track decoded instructions (#1320)
  Fix polkadot-node-core-pvf-prepare-worker build with jemalloc (#1315)
  Sassafras primitives (#1249)
  Restructure `dispatch` macro related exports (#1162)
  backing: move the min votes threshold to the runtime (#1200)
  Bump zstd from 0.11.2+zstd.1.5.2 to 0.12.4 (#1326)
  Remove `substrate_test_utils::test` (#1321)
  remove disable-runtime-api (#1328)
  [ci] add more jobs for pipeline cancel, cleanup (#1314)
  ...
ordian added a commit that referenced this pull request Sep 7, 2023
@ordian
Merge branch 'master' into ao-polkadot-use-pinning-api
d3e00e0 
* master: (25 commits)
  Markdown linter (#1309)
  Update `fmt` file and some authors (#1379)
  Bump the known_good_semver group with 1 update (#1375)
  Bump proc-macro-warning from 0.4.1 to 0.4.2 (#1376)
  feat: add futures api to `TransactionPool` (#1348)
  Ensure cumulus/bridges is ignored by formatter and run it (#1369)
  substrate: chain-spec paths corrected in zombienet tests (#1362)
  contracts: Update to wasmi 0.31 (#1350)
  [improve docs]: Template pallet (#1280)
  [xcm-emulator] Unignore cumulus integration tests (#1247)
  Fix wrong ref counting (#1358)
  Use cached session index to obtain executor params (#1190)
  fix typos (#1339)
  Use bandersnatch-vrfs with locked dependencies ref (#1342)
  Bump bs58 from 0.4.0 to 0.5.0 (#1293)
  Contracts: `seal0::balance` should return the free balance (#1254)
  Logs: add extra debug log for negative rep changes (#1205)
  Added short-benchmarks for cumulus (#1183)
  [xcm-emulator] Improve hygiene and clean up (#1301)
  Bump the known_good_semver group with 1 update (#1347)
  ...
Daanvdplas pushed a commit that referenced this pull request Sep 11, 2023
@bkontur @alvicsam
@pepoviola @Daanvdplas
Added short-benchmarks for cumulus (#1183)
02bbde6 
* Added short-benchmarks for cumulus

* Added `--bin` flag for short-benchmarks

* fix dependency for short-benchmark-cumulus

* Fixed benchmark with new XCM::V3 `MAX_INSTRUCTIONS_TO_DECODE`

* Fixed benchmark for bridge messages pallets

---------

Co-authored-by: alvicsam <alvicsam@gmail.com>
Co-authored-by: Javier Viola <javier@parity.io>
Co-authored-by: Alexander Samusev <41779041+alvicsam@users.noreply.github.com>
@crystalin crystalin mentioned this pull request Oct 20, 2023
Closed
bkchr pushed a commit that referenced this pull request Apr 10, 2024
@svyatonik @bkchr
fix/ignore cargo deny issues (#1183)
f4a82d4
This was referenced Jun 5, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bkchr bkchr bkchr approved these changes

@alvicsam alvicsam alvicsam approved these changes

+1 more reviewer

@sergejparity sergejparity sergejparity approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A3-backport Pull request is already reviewed well in another branch. T9-cumulus This PR/Issue is related to cumulus. T12-benchmarks This PR/Issue is related to benchmarking and weights. T14-system_parachains This PR/Issue is related to system parachains.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@bkontur @gilescope @bkchr @alvicsam @sergejparity @patriciobcs @pepoviola