Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[omni-bencher] Make all runtimes work #5872

Merged
merged 48 commits into from
Oct 8, 2024
Merged

[omni-bencher] Make all runtimes work #5872

merged 48 commits into from
Oct 8, 2024

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Sep 30, 2024

Changes:

  • Add --exclude-pallets to exclude some pallets from runtimes where we dont have genesis presets yet
  • Make --genesis-builder-policy=none work with --runtime
  • CI: Run the frame-omni-bencher for all runtimes

ggwpez and others added 30 commits September 9, 2024 19:34
@ggwpez
Set genesis state after each benchmark run
3631111 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
Add test
a2cc75a 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez @github-actions
Add PrDoc (auto generated)
50ce303
@ggwpez
Use DEV_RUNTIME_PRESET instead of hard-coding
6e87e57 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
INSTALL WITH LOCKED FFS
9aa865f 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
Revert "INSTALL WITH LOCKED FFS"
6bb2cb5 
This reverts commit 9aa865f.
@ggwpez
Only test in std
c846cf5 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
omni-bencher: Fix runtime genesis generation
8809ce5 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez @bkchr
Update substrate/utils/frame/benchmarking-cli/src/pallet/command.rs
a9ec975 
Co-authored-by: Bastian Köcher <[email protected]>
@ggwpez @bkchr
Update substrate/utils/frame/benchmarking-cli/src/pallet/command.rs
f205230 
Co-authored-by: Bastian Köcher <[email protected]>
@ggwpez @bkchr
Update substrate/utils/frame/benchmarking-cli/src/pallet/command.rs
669c88a 
Co-authored-by: Bastian Köcher <[email protected]>
@ggwpez @bkchr
Update substrate/utils/frame/benchmarking-cli/src/pallet/command.rs
a539e2a 
Co-authored-by: Bastian Köcher <[email protected]>
@ggwpez
Stay backwards compatible
f641517 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
Rename to GenesisBuilderPolicy
1da1213 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
Merge branch 'master' into oty-omni-bencher-fix
b98d341
@ggwpez
clippy
68413fa 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
clippy
8b97944 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
clippy
41cb2bf 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
fix bench test
b3e8e15 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@bkontur
Merge branch 'master' into oty-omni-bencher-fix
f7bfa0a
@bkontur
Enable also new GHA for short-benchmarks
4fd7581
@alvicsam
use correct runner for benches
516ca5c
@bkontur
Merge branch 'master' into bko-enable-gha-short-benchmarks
26d9379
@ggwpez
[omni-bencher] Make work with --runtime and genesis-builder-policy=none
96540cb 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
[omni-bencher] Add --exclude-pallets option
f5f67d6 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
CI use matrix for args
2256664 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
Merge remote-tracking branch 'origin/bko-enable-gha-short-benchmarks'…
59fc11f 
… into oty-omni-bencher-fix2
@ggwpez
post merge fixes
d18d0a7 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
Merge branch 'master' into oty-omni-bencher-fix2
30ee680
@ggwpez
fix ci
82a1ca8 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez ggwpez marked this pull request as ready for review October 2, 2024 10:40
@ggwpez ggwpez requested review from a team as code owners October 2, 2024 10:40
@ggwpez ggwpez requested review from bkontur and skunert October 2, 2024 10:40
@ggwpez ggwpez added the T12-benchmarks This PR/Issue is related to benchmarking and weights. label Oct 2, 2024
@ggwpez
prdoc
00f7e9d 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
skunert
skunert reviewed Oct 2, 2024
View reviewed changes
Copy link
Contributor

@skunert skunert left a comment

Choose a reason for hiding this comment

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

One thing I noticed is that we currently have no way to use the default preset of any runtime. You can only specify a specific patch or nothing at all.

.github/workflows/runtimes-matrix.json
"template": "cumulus/templates/xcm-bench-template.hbs",
"bench_features": "runtime-benchmarks",
"bench_flags": "--genesis-builder-policy=none --exclude-pallets=pallet_xcm",
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I have to say I don't really understand why we are using genesis-builder-policy=none. It will use an empty genesis, why do we want that?

Copy link
Member Author

Choose a reason for hiding this comment

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

We use none for the runtimes that dont yet support the genesis builder pattern. It is just to make the benchmarks run at all, otherwise it just errors that there are no presets. That is why i only added it to some runtimes.

Copy link
Member Author

Choose a reason for hiding this comment

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

After DMin i will try it with the chain specs, maybe those work.

Copy link
Member Author

Choose a reason for hiding this comment

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

The specs are not compiled with the benchmarking features, so that does not work either.

@paritytech-review-bot paritytech-review-bot bot requested a review from a team October 4, 2024 20:58
alvicsam
alvicsam approved these changes Oct 7, 2024
View reviewed changes
Copy link
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.

A few nits in ci

.github/workflows/check-frame-omni-bencher.yml Outdated Show resolved Hide resolved
.github/workflows/check-frame-omni-bencher.yml Outdated Show resolved Hide resolved
ggwpez added 2 commits October 7, 2024 12:59
@ggwpez
Add log target and better error message
63125fe 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
clippy
2e49563 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez ggwpez added this pull request to the merge queue Oct 8, 2024
Merged via the queue into master with commit c482333 Oct 8, 2024
221 checks passed
@ggwpez ggwpez deleted the oty-omni-bencher-fix2 branch October 8, 2024 14:19
@alvicsam
Copy link
Contributor

alvicsam commented Oct 8, 2024

Can we remove short benchmarks from gitlab?

@mordamax
Copy link
Contributor

mordamax commented Oct 8, 2024

Can we remove short benchmarks from gitlab?

i think yes

image https://github.com/paritytech/polkadot-sdk/actions/runs/11239542155

@bkontur bkontur mentioned this pull request Oct 9, 2024
Closed
ordian added a commit that referenced this pull request Oct 11, 2024
@ordian
Merge branch 'master' into ao-fix-issue-64
2b29d09 
* master: (28 commits)
  `substrate-node`: removed excessive polkadot-sdk features (#5925)
  Rename QueueEvent::StartWork (#6015)
  [ci] Remove quick-benchmarks-omni from GitLab (#6014)
  Set larger timeout for cmd.yml (#6006)
  Fix `0003-beefy-and-mmr` test (#6003)
  Remove redundant XCMs from dry run's forwarded xcms (#5913)
  Add RadiumBlock bootnodes to Coretime Polkadot Chain spec (#5967)
  Bump strum from 0.26.2 to 0.26.3 (#5943)
  Add PVF execution priority (#4837)
  Snowbridge V2 docs (#5902)
  Fix u256 conversion in BABE (#5994)
  [ci] Move test-linux-stable-no-try-runtime to GHA (#5979)
  Bump PoV request timeout (#5924)
  [Release/CI] Github flow to build `polkadot`/`polkadot-parachain` rc binaries and deb package (#5963)
  [ci] Remove short-benchmarks from Gitlab (#5988)
  Disable flaky tests reported in 5972/5973/5974 (#5976)
  Bump some dependencies (#5886)
  bump zombienet version and set request for k8s (#5968)
  [omni-bencher] Make all runtimes work (#5872)
  Omni-Node renamings (#5915)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skunert skunert skunert left review comments

@mordamax mordamax mordamax approved these changes

@alvicsam alvicsam alvicsam approved these changes

@kianenigma kianenigma kianenigma approved these changes

@bkontur bkontur Awaiting requested review from bkontur

Assignees
No one assigned
Labels
T12-benchmarks This PR/Issue is related to benchmarking and weights.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ggwpez @alvicsam @mordamax @kianenigma @skunert @bkontur