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

Replace all manual weight accounting with benchmark-derived functions #1190

Closed
4meta5 opened this issue Oct 7, 2024 · 1 comment · Fixed by #1219
Closed

Replace all manual weight accounting with benchmark-derived functions #1190

4meta5 opened this issue Oct 7, 2024 · 1 comment · Fixed by #1219
Assignees
Labels
bug Something isn't working priority: critical

Comments

@4meta5
Copy link
Contributor

4meta5 commented Oct 7, 2024

Noticed during #1185 that pallet elections uses manual weight accounting in a few places. All manual weight accounting should be replaced with benchmark-derived functions.

  • elections::try_elect_shard
  • elections::member_offline
  • shards::member_offline
  • shards::on_initialize
  • check ALL other pallets for any other manual weight accounting worth updating to the benchmark-derived function

The benchmark-derived weight function approach is preferred to manual accounting for 2 reasons:

  1. easier to maintain because the code does not have to updated manually with changes
  2. more accurate because it accounts for more than just the unit cost of reads/writes
@4meta5 4meta5 added the bug Something isn't working label Oct 7, 2024
@4meta5 4meta5 changed the title remove all manual weight accounting Replace all manual weight accounting with benchmark-derived functions Oct 7, 2024
@4meta5 4meta5 self-assigned this Oct 7, 2024
@4meta5 4meta5 added this to the Milestone Q4 2024 milestone Oct 7, 2024
@4meta5
Copy link
Contributor Author

4meta5 commented Oct 18, 2024

Increased priority and started immediately because I want weight accounting to be as accurate as possible to decrease the likelihood that inaccurate weights are contributing to inconsistent block times (which is sometimes the case when inaccuracy underestimates actual computation).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: critical
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant