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

Add overhead benchmark to polkadot-parachain #2900

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add overhead benchmark to polkadot-parachain #2900

wants to merge 5 commits into from

Conversation

kalaninja
Copy link

@kalaninja kalaninja commented Jul 19, 2023 •
edited
Loading

Should be usable with command bot

@paritytech-ci paritytech-ci requested review from a team July 19, 2023 06:10
@kalaninja kalaninja requested review from bkchr and ggwpez July 19, 2023 06:11
@kalaninja kalaninja mentioned this pull request Jul 19, 2023
Open
@bkontur bkontur mentioned this pull request Jul 19, 2023
Merged
@kalaninja kalaninja requested a review from bkontur July 19, 2023 11:50
ggwpez
ggwpez reviewed Jul 19, 2023
View reviewed changes
polkadot-parachain/tests/benchmark_overhead_works.rs
.arg(base_path)
.args(["--warmup", "10", "--repeat", "10"])
.args(["--add", "100", "--mul", "1.2", "--metric", "p75"])
.args(["--max-ext-per-block", "10"])
Copy link
Member

Choose a reason for hiding this comment

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

Depending on CI timing we can reduce these numbers a bit here and for warmup and repeat but it should be rather fast.
The slowest part is actually building the block.

@ggwpez ggwpez added A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. T0-node This PR/Issue is related to the topic “node”. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jul 19, 2023
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable
Logs: https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3240827

ggwpez
ggwpez approved these changes Jul 22, 2023
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.

LGTM besides the failing test with "Bad input data provided to apply_extrinsic: Codec error" so something with the signed extensions is maybe wrong? (Good that you added the tests 😆).

@ggwpez ggwpez mentioned this pull request Jul 22, 2023
Open
@kalaninja
Copy link
Author

@ggwpez Fixed. Good that among two of at least one knows that tests should be run after refactoring. 😆

@kalaninja kalaninja force-pushed the polkadot-para-overhead-bench branch from 391a41c to 9db0859 Compare July 23, 2023 14:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ggwpez ggwpez ggwpez approved these changes

@bkchr bkchr Awaiting requested review from bkchr

@bkontur bkontur Awaiting requested review from bkontur

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the 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 T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kalaninja @paritytech-cicd-pr @bkontur @ggwpez