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.

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

Update pallet weights every week #6196

Open
ggwpez opened this issue Oct 23, 2024 · 3 comments · May be fixed by #6789
Open

Update pallet weights every week #6196

ggwpez opened this issue Oct 23, 2024 · 3 comments · May be fixed by #6789
Assignees

Comments

@ggwpez
Copy link
Member

ggwpez commented Oct 23, 2024

With changes to FRAME, the Rust compiler version, pallets and the benchmarker itself, we should try to update the pallet weights on a weekly basis to see regressions early and not just before a release.

Task: Update the weights of all pallets from all runtimes every week. It should re-use the already open MR and force-push there, to avoid spamming up new MRs if reviews are slow.

@mordamax mordamax self-assigned this Oct 23, 2024
@bkchr
Copy link
Member

bkchr commented Oct 23, 2024

Task: Update the weights of all pallets from all runtimes every week. It should re-use the already open MR and force-push there, to avoid spamming up new MRs if reviews are slow.

No no. These prs should be merged automatically if the weight changes are not a regression. Generally I would also expect if these kind of prs exist they should also get merged. No need to bring this stuff up and then it lays around.

@mordamax
Copy link
Contributor

First iteration anyway is not going to go automatically, but through a review, at least to establish the baseline, as many of the weights either do not exist or very outdated

https://github.com/paritytech/polkadot-sdk/pull/6789/files
This workflow allows us to start (or restart) this as we want through workflow dispatch, and will create a PR with all runtimes recalculated diff + subweights report so it's easier to scan visually the outsiders.

The next step would be finding out how could we further automate it so it will require minimum of the developer intervention, ideally.

for now - My pov is that it should always go through at least 1 person, may be CI/RelEng or any core-dev. As i'd be very hesitant to trust any kind of tooling a weights pushed directly to master without human checking. Imagine CI/tooling bug which ends up with too low or crazy high numbers weights in master 🤯

May be adding some enhancements like bot pre-checking if it's all good or something is bad, or highlights the pallet outsiders, which would help developer to spot the degradation, and/or enable auto-merge in PR, but still a human to approve

@bkchr
Copy link
Member

bkchr commented Jan 17, 2025

Imagine CI/tooling bug which ends up with too low or crazy high numbers weights in master 🤯

No one should use the weights in production...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants