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

Deprecate Weight::from_{ref_time, proof_size} #13475

Merged
merged 11 commits into from
Mar 2, 2023

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Feb 27, 2023

Changes:

  • Deprecate Weight::from_ref_time and Weight::from_proof_size and use from_parts instead
  • Update weight templates

TODO:

  • Update weight files
  • Double-check templates

Polkadot companion: paritytech/polkadot#6794
Cumulus companion: paritytech/cumulus#2245
Migration script for down-stream projects: migrate-from-parts.py.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
Copy link
Member Author

ggwpez commented Feb 27, 2023

bot bench $ pallet dev pallet_balances

@command-bot
Copy link

command-bot bot commented Feb 27, 2023

@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2453130 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_balances. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 102-4507782f-6044-45a3-af37-232a5087f28d to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Feb 27, 2023

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_balances has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2453130 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2453130/artifacts/download.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez ggwpez marked this pull request as ready for review February 27, 2023 12:56
@ggwpez ggwpez added A0-please_review Pull request needs code review. 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 B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. T2-API This PR/Issue is related to APIs. labels Feb 27, 2023
@ggwpez ggwpez removed the T2-API This PR/Issue is related to APIs. label Feb 27, 2023
{{#each benchmark.component_weight as |cw|}}
// Standard Error: {{underscore cw.error}}
.saturating_add(Weight::from_ref_time({{underscore cw.slope}}).saturating_mul({{cw.name}}.into()))
.saturating_add(Weight::from_parts({{underscore cw.slope}}, 0).saturating_mul({{cw.name}}.into()))
Copy link
Member

Choose a reason for hiding this comment

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

why are we ever using 0 for proof size?

@@ -72,7 +71,7 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
.saturating_add(T::DbWeight::get().writes(({{cw.slope}}_u64).saturating_mul({{cw.name}}.into())))
{{/each}}
{{#each benchmark.component_calculated_proof_size as |cp|}}
.saturating_add(Weight::from_proof_size({{cp.slope}}).saturating_mul({{cp.name}}.into()))
.saturating_add(Weight::from_parts(0, {{cp.slope}}).saturating_mul({{cp.name}}.into()))
Copy link
Member

Choose a reason for hiding this comment

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

same - why 0 for ref time?

Comment on lines 131 to 136
.saturating_add(Weight::from_parts(13_799_167, 0).saturating_mul(c.into()))
.saturating_add(T::DbWeight::get().reads(2_u64))
.saturating_add(T::DbWeight::get().reads((2_u64).saturating_mul(c.into())))
.saturating_add(T::DbWeight::get().writes(1_u64))
.saturating_add(T::DbWeight::get().writes((2_u64).saturating_mul(c.into())))
.saturating_add(Weight::from_proof_size(5180).saturating_mul(c.into()))
.saturating_add(Weight::from_parts(0, 5180).saturating_mul(c.into()))
Copy link
Member

Choose a reason for hiding this comment

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

why do these two not just get combined into a single from_parts?

Copy link
Member Author

Choose a reason for hiding this comment

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

The template is a bit inflexible right now, I can try but it will not look much better. Having these written as a term is very poor choice in retrospect.
Something data-based would be better like https://github.com/paritytech/substrate/issues/12580#issuecomment-1304662025

@ggwpez
Copy link
Member Author

ggwpez commented Mar 2, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 6e81199 into master Mar 2, 2023
@paritytech-processbot paritytech-processbot bot deleted the oty-deprecate-weight-from branch March 2, 2023 21:28
deepink-mas added a commit to deep-ink-ventures/genesis-dao-node that referenced this pull request Mar 29, 2023
@nazar-pc nazar-pc mentioned this pull request Apr 2, 2023
1 task
ukint-vs pushed a commit to gear-tech/substrate that referenced this pull request Apr 10, 2023
* Deprecate Weight::from_{ref_time, proof_size}

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Update templates

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Use from_parts

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Use from_parts

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Dont revert comment 🤦

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_balances

* Update weight files

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* More fixes

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Adapt to Master changes

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: command-bot <>
OverOrion added a commit to OverOrion/pallets that referenced this pull request May 26, 2023
OverOrion added a commit to integritee-network/pallets that referenced this pull request May 31, 2023
* polkadot update to v0.9.42

* remove deprecated trait Store uses

See paritytech/substrate#13535

* remove deprecated Weight::from_{ref_time, proof_size}

See paritytech/substrate#13475

* allowlist one constant weight

See paritytech/substrate#13798

* add new trait associated types

* polkadot update to v0.9.42 (xcm)

* fixup! remove deprecated trait Store uses

* teerex/mock: add missing trait imports

* claims/tests: fix benchmark as described in substrate/12951

See: paritytech/substrate#12951

* claims/tests: fix claiming_while_vested_doesnt_work by giving it the existential deposit (ED)

Co-authored-by: coax1d <[email protected]>

---------

Co-authored-by: coax1d <[email protected]>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* Deprecate Weight::from_{ref_time, proof_size}

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Update templates

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Use from_parts

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Use from_parts

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Dont revert comment 🤦

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_balances

* Update weight files

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* More fixes

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Adapt to Master changes

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: command-bot <>
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. 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 T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants