[benchmarking] Reset to genesis storage after each run #5655

ggwpez · 2024-09-09T17:37:08Z

The genesis state is currently partially provided via OverlayedChanges, but these changes are reset by the runtime after the first repetition, causing the second repitition to use an invalid genesis state.

Changes:

Provide the genesis state as a Storage without any OverlayedChanges to make it work correctly with repetitions.
Add --genesis-builder-preset option to use different genesis preset names.
Improve error messages.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs

bkontur

lgtm

I will rebase #5327 tomorrow.
There is commit with enabling of the new short benchmarking CI with frame-omni-bencher, so BridgeHubRococo with frame-omni-bencher fails there, so after rebase it should work.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

This reverts commit 9aa865f.

bkchr · 2024-09-12T21:09:54Z

prdoc/pr_5655.prdoc

+doc:
+- audience: Runtime Dev
+  description: |-
+    The genesis state is currently partially provided via `OverlayedChanges`, but these changes are reset by the runtime after the first repetition, causing the second repitition to use an invalid genesis state.


but these changes are reset by the runtime after the first repetition

You mean here that this was overridden and not reset?

substrate/utils/frame/benchmarking-cli/src/pallet/command.rs

skunert

Okay this is funny, did not see this PR but had similar thoughts today: #5694

While at it, should we fix issue 2a from the linked issue too?

skunert · 2024-09-12T21:20:51Z

substrate/utils/frame/benchmarking-cli/src/pallet/command.rs

 		})
 	}

+	/// Convert some overlayed changes to a storage.
+	fn changes_to_storage<H: Hash>(


Why not get rid of all of this and genesis_from_runtime and just use GenesisConfigBuilderRuntimeCaller? It has all the machinery ready and we get rid of the manual json merging stuff too.

I'd also try using GenesisConfigBuilderRuntimeCaller to have genesis storage building in one place. If something is missing it can be adjusted / reworked.

I changed it now.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

substrate/utils/frame/benchmarking-cli/src/pallet/types.rs

skunert · 2024-09-19T12:35:47Z

@ggwpez Anything speaks against merging this? I have a follow up that could share some code.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

It is a first step for switching to the `frame-omni-bencher` for CI. This PR includes several changes related to generating chain specs plus: - [x] pallet `assigned_slots` fix missing `#[serde(skip)]` for phantom - [x] pallet `paras_inherent` benchmark fix - cherry-picked from #5688 - [x] migrates `get_preset` to the relevant runtimes - [x] fixes Rococo genesis presets - does not work https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7317249 - [x] fixes Rococo benchmarks for CI - [x] migrate westend genesis - [x] remove wococo stuff Closes: #5680 ## Follow-ups - Fix for frame-omni-bencher #5655 - Enable new short-benchmarking CI - #5706 - Remove gitlab pipelines for short benchmarking - refactor all Cumulus runtimes to use `get_preset` - #5704 - #5705 - #5700 - [ ] Backport to the stable --------- Co-authored-by: command-bot <> Co-authored-by: ordian <[email protected]>

It is a first step for switching to the `frame-omni-bencher` for CI. This PR includes several changes related to generating chain specs plus: - [x] pallet `assigned_slots` fix missing `#[serde(skip)]` for phantom - [x] pallet `paras_inherent` benchmark fix - cherry-picked from #5688 - [x] migrates `get_preset` to the relevant runtimes - [x] fixes Rococo genesis presets - does not work https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7317249 - [x] fixes Rococo benchmarks for CI - [x] migrate westend genesis - [x] remove wococo stuff Closes: #5680 - Fix for frame-omni-bencher #5655 - Enable new short-benchmarking CI - #5706 - Remove gitlab pipelines for short benchmarking - refactor all Cumulus runtimes to use `get_preset` - #5704 - #5705 - #5700 - [ ] Backport to the stable --------- Co-authored-by: command-bot <> Co-authored-by: ordian <[email protected]> (cherry picked from commit 8735c66)

prdoc/pr_5655.prdoc

bkontur · 2024-09-22T19:05:49Z

This fix is working finally for short-benchmarking with omni-bencher for bridge-hubs/collectives:
https://github.com/paritytech/polkadot-sdk/actions/runs/10983379680/job/30492700127?pr=5706
https://github.com/paritytech/polkadot-sdk/actions/runs/10983379680/job/30492700240?pr=5706
https://github.com/paritytech/polkadot-sdk/actions/runs/10983379680/job/30492700384?pr=5706
so merging this

paritytech-cmd-bot-polkadot-sdk · 2024-09-22T23:04:03Z

Created backport PR for stable2407:

[stable2407] Backport #5655 #5799 with remaining conflicts!

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-5655-to-stable2407
git worktree add --checkout .worktree/backport-5655-to-stable2407 backport-5655-to-stable2407
cd .worktree/backport-5655-to-stable2407
git reset --hard HEAD^
git cherry-pick -x 2e4e5bf2fd0ae19fa38951c7e5f495dd1453b2bb
git push --force-with-lease

The genesis state is currently partially provided via `OverlayedChanges`, but these changes are reset by the runtime after the first repetition, causing the second repitition to use an invalid genesis state. Changes: - Provide the genesis state as a `Storage` without any `OverlayedChanges` to make it work correctly with repetitions. - Add `--genesis-builder-preset` option to use different genesis preset names. - Improve error messages. --------- Signed-off-by: Oliver Tale-Yazdi <[email protected]> Co-authored-by: ggwpez <[email protected]> Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: Branislav Kontur <[email protected]> (cherry picked from commit 2e4e5bf)

paritytech-cmd-bot-polkadot-sdk · 2024-09-22T23:04:08Z

Successfully created backport PR for stable2409:

[stable2409] Backport #5655 #5800

Backport #5655 into `stable2409` from ggwpez. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot.  --------- Signed-off-by: Oliver Tale-Yazdi <[email protected]> Co-authored-by: Oliver Tale-Yazdi <[email protected]>

bkontur · 2024-09-30T12:11:23Z

@ggwpez Oliver, we probably didn’t realize this earlier, but with this PR, frame-omni-bencher now requires get_preset("development") from the runtime. Otherwise, the benchmarking fails. This creates a problem for runtimes that do not implement get_preset, e.g., Kitchensink is missing it, and several testnet runtimes are now failing: #5706 — due to the missing get_preset("development"). Here's the issue to address this. The question is, should omni-bencher be this strict, or should it work with a get_preset -> None result?

cc: @mordamax

ggwpez · 2024-11-25T07:44:50Z

We have a flag to make it run with None, but the benchmarks themselves are possibly not written in a way that this would work.

Set genesis state after each benchmark run

3631111

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

ggwpez marked this pull request as draft September 9, 2024 17:37

Add test

a2cc75a

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

ggwpez added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Sep 9, 2024

ggwpez marked this pull request as ready for review September 9, 2024 18:10

ggwpez requested a review from a team as a code owner September 9, 2024 18:10

Add PrDoc (auto generated)

50ce303

ggwpez requested a review from bkontur September 9, 2024 19:30

bkontur reviewed Sep 9, 2024

View reviewed changes

substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs Outdated Show resolved Hide resolved

bkontur approved these changes Sep 9, 2024

View reviewed changes

ggwpez added 2 commits September 10, 2024 16:40

Use DEV_RUNTIME_PRESET instead of hard-coding

6e87e57

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

INSTALL WITH LOCKED FFS

9aa865f

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

ggwpez requested review from a team as code owners September 10, 2024 17:10

Revert "INSTALL WITH LOCKED FFS"

6bb2cb5

This reverts commit 9aa865f.

ggwpez removed request for a team September 10, 2024 17:13

This was referenced Sep 11, 2024

Moved presets to the testnet runtimes #5327

Merged

use RuntimeGenesisConfig in genesis config presets polkadot-fellows/runtimes#451

Open

bkchr approved these changes Sep 12, 2024

View reviewed changes

bkchr reviewed Sep 12, 2024

View reviewed changes

substrate/utils/frame/benchmarking-cli/src/pallet/command.rs Show resolved Hide resolved

skunert reviewed Sep 12, 2024

View reviewed changes

bkontur mentioned this pull request Sep 13, 2024

Enable GHA short benchmarks with frame-omni-bencher #5706

Closed

bkontur added the A4-needs-backport Pull request must be backported to all maintained releases. label Sep 13, 2024

This was referenced Sep 13, 2024

polkadot-omni-node: Fully remove benchmark sub-command #5564

Open

CI short benchmarking with frame-omni-bencher #4405

Closed

[CI] Integrate Runtime Omni Bencher AstarNetwork/Astar#1316

Closed

ggwpez added 2 commits September 13, 2024 18:15

Only test in std

c846cf5

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

omni-bencher: Fix runtime genesis generation

8809ce5

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

ggwpez requested a review from skunert September 13, 2024 17:14

michalkucharczyk approved these changes Sep 17, 2024

View reviewed changes

michalkucharczyk reviewed Sep 17, 2024

View reviewed changes

substrate/utils/frame/benchmarking-cli/src/pallet/types.rs Outdated Show resolved Hide resolved

ggwpez added 6 commits September 21, 2024 00:06

Rename to GenesisBuilderPolicy

1da1213

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

Merge branch 'master' into oty-omni-bencher-fix

b98d341

clippy

68413fa

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

clippy

8b97944

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

clippy

41cb2bf

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

fix bench test

b3e8e15

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

Merge branch 'master' into oty-omni-bencher-fix

f7bfa0a

bkontur reviewed Sep 22, 2024

View reviewed changes

prdoc/pr_5655.prdoc Outdated Show resolved Hide resolved

Update prdoc/pr_5655.prdoc

74b6a13

Merge branch 'master' into oty-omni-bencher-fix

d83867a

bkontur enabled auto-merge September 22, 2024 21:56

bkontur added this pull request to the merge queue Sep 22, 2024

Merged via the queue into master with commit 2e4e5bf Sep 22, 2024
207 of 209 checks passed

bkontur deleted the oty-omni-bencher-fix branch September 22, 2024 23:03

paritytech-cmd-bot-polkadot-sdk bot mentioned this pull request Sep 22, 2024

[stable2407] Backport #5655 #5799

Closed

paritytech-cmd-bot-polkadot-sdk bot mentioned this pull request Sep 22, 2024

[stable2409] Backport #5655 #5800

Merged

TDemeco mentioned this pull request Oct 14, 2024

feat: ⏫ upgrade to Polkadot SDK stable2409 Moonsong-Labs/storage-hub#228

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[benchmarking] Reset to genesis storage after each run #5655

[benchmarking] Reset to genesis storage after each run #5655

ggwpez commented Sep 9, 2024 •

edited

Loading

bkontur left a comment

bkchr Sep 12, 2024

skunert left a comment

skunert Sep 12, 2024

michalkucharczyk Sep 13, 2024

ggwpez Sep 13, 2024

skunert commented Sep 19, 2024

bkontur commented Sep 22, 2024

paritytech-cmd-bot-polkadot-sdk bot commented Sep 22, 2024

paritytech-cmd-bot-polkadot-sdk bot commented Sep 22, 2024

bkontur commented Sep 30, 2024 •

edited

Loading

ggwpez commented Nov 25, 2024

[benchmarking] Reset to genesis storage after each run #5655

[benchmarking] Reset to genesis storage after each run #5655

Conversation

ggwpez commented Sep 9, 2024 • edited Loading

bkontur left a comment

Choose a reason for hiding this comment

bkchr Sep 12, 2024

Choose a reason for hiding this comment

skunert left a comment

Choose a reason for hiding this comment

skunert Sep 12, 2024

Choose a reason for hiding this comment

michalkucharczyk Sep 13, 2024

Choose a reason for hiding this comment

ggwpez Sep 13, 2024

Choose a reason for hiding this comment

skunert commented Sep 19, 2024

bkontur commented Sep 22, 2024

paritytech-cmd-bot-polkadot-sdk bot commented Sep 22, 2024

paritytech-cmd-bot-polkadot-sdk bot commented Sep 22, 2024

bkontur commented Sep 30, 2024 • edited Loading

ggwpez commented Nov 25, 2024

ggwpez commented Sep 9, 2024 •

edited

Loading

bkontur commented Sep 30, 2024 •

edited

Loading