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

Frame System Benchmarking #5834

Merged
merged 5 commits into from
Apr 30, 2020
Merged

Frame System Benchmarking #5834

merged 5 commits into from
Apr 30, 2020

Conversation

shawntabrizi
Copy link
Member

This PR adds a crate that benchmarks the various extrinsics in frame_system.

@shawntabrizi shawntabrizi added this to the 2.0 milestone Apr 30, 2020
@shawntabrizi shawntabrizi added the A0-please_review Pull request needs code review. label Apr 30, 2020
Copy link
Contributor

@apopiak apopiak left a comment

Choose a reason for hiding this comment

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

Except for the prefix length maybe being relevant:
LGTM, but of course I might have missed sth because I'm not familir enough with the system.

frame/system/benchmarking/Cargo.toml Show resolved Hide resolved
frame/system/benchmarking/src/lib.rs Show resolved Hide resolved
frame/system/benchmarking/src/lib.rs Show resolved Hide resolved
frame/system/benchmarking/src/lib.rs Show resolved Hide resolved
frame/system/benchmarking/src/lib.rs Show resolved Hide resolved
frame/system/benchmarking/src/lib.rs Outdated Show resolved Hide resolved
frame/system/benchmarking/src/lib.rs Outdated Show resolved Hide resolved
Comment on lines +52 to +54
// `set_code` was not benchmarked because it is pretty hard to come up with a real
// Wasm runtime to test the upgrade with. But this is okay because we will make
// `set_code` take a full block anyway.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to benchmark (more, like, smoketest) whether you can find a WASM that would take longer than the block time to call set_code on. Not sure how to do that, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would have to be something we test more manually or with some other setup versus using our runtime benchmarking

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe open an issue, would be good to follow up

Copy link
Member

Choose a reason for hiding this comment

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

Basically, they all have an effective weight higher than a block. Just something we have to live with. It's a root call so still basically fine.

frame/system/benchmarking/src/lib.rs Outdated Show resolved Hide resolved
frame/system/benchmarking/src/mock.rs Outdated Show resolved Hide resolved
@apopiak apopiak self-requested a review April 30, 2020 09:04
Copy link
Contributor

@apopiak apopiak left a comment

Choose a reason for hiding this comment

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

LGTM

@gavofyork gavofyork merged commit 6d8ddd4 into master Apr 30, 2020
@gavofyork gavofyork deleted the shawntabrizi-bench-system branch April 30, 2020 13:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants