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

Fix Gov V2 Benchmarks #12022

Merged
merged 6 commits into from
Aug 13, 2022
Merged

Conversation

shawntabrizi
Copy link
Member

@shawntabrizi shawntabrizi commented Aug 12, 2022

Benchmarks for Governance V2 were failing: paritytech/polkadot#5205

This should fix them.

Fixes: #11906

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Aug 12, 2022
@ggwpez
Copy link
Member

ggwpez commented Aug 12, 2022

Does this fix all of them since they are related?
I recorded three pallets as failing: #11906

@shawntabrizi
Copy link
Member Author

@ggwpez okay might need to check the other pallets

@shawntabrizi
Copy link
Member Author

shawntabrizi commented Aug 13, 2022

@ggwpez the other failing pallets are fixed by updating the Polkadot configuration, which I have done. This should be all of them I think.

@shawntabrizi shawntabrizi added B0-silent Changes should not be mentioned in any 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 labels Aug 13, 2022
Comment on lines +702 to +703
let when = when.saturating_add(alarm_interval).saturating_sub(One::one()) /
(alarm_interval.saturating_mul(alarm_interval)).max(One::one());
Copy link
Member Author

Choose a reason for hiding this comment

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

unsafe math here caused a panic in benchmarks

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.

Should I diener and test again or are you confident that it works?

@shawntabrizi
Copy link
Member Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit d558a21 into master Aug 13, 2022
@paritytech-processbot paritytech-processbot bot deleted the shawntabrizi-benchmark-log branch August 13, 2022 12:05
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* better benchmark log

* Update lib.rs

* fix gov2 benchmarks

* saturating math + fmt

* add comment
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. B0-silent Changes should not be mentioned in any 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
Projects
Development

Successfully merging this pull request may close these issues.

Fix Governance v2 benchmarks
4 participants