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

Timestamp weights #5775

Merged
13 commits merged into from
Apr 30, 2020
Merged

Timestamp weights #5775

13 commits merged into from
Apr 30, 2020

Conversation

apopiak
Copy link
Contributor

@apopiak apopiak commented Apr 24, 2020

Add weight calculation to TimeStamp::set and return weight from dummy on_initialize.
Note that it is assumed that DidUpdate does not incur any database costs because it is cleared before the end of the block.

fix one out-standing change in Identity (see here) as well as its weights.

related to #5596

@parity-cla-bot
Copy link

It looks like @apopiak signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@apopiak apopiak requested a review from kianenigma April 24, 2020 15:22
@apopiak apopiak marked this pull request as ready for review April 24, 2020 15:25
frame/timestamp/src/lib.rs Outdated Show resolved Hide resolved
@apopiak
Copy link
Contributor Author

apopiak commented Apr 28, 2020 via email

@apopiak apopiak requested review from shawntabrizi and gui1117 April 29, 2020 10:00
@gavofyork gavofyork added the A0-please_review Pull request needs code review. label Apr 29, 2020
@gavofyork gavofyork added A8-mergeoncegreen and removed A0-please_review Pull request needs code review. labels Apr 29, 2020
@gnunicorn
Copy link
Contributor

@apopiak test needs updating.

@apopiak
Copy link
Contributor Author

apopiak commented Apr 29, 2020

@gnunicorn Any idea how this could have been caused by my changes?

@gnunicorn
Copy link
Contributor

gnunicorn commented Apr 29, 2020

@apopiak check the logs. Seems like the weight has changed between the found and expected results:

 test full_native_block_import_works ... FAILED
   left: `[EventRecord { phase: Phase::ApplyExtrinsic(0), event: Event::frame_system(RawEvent::ExtrinsicSuccess(DispatchInfo { weight: 348000000, class: DispatchClass::Mandatory, pays_fee: Pays::Yes })), topics: [] }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: Event::pallet_balances(RawEvent::Transfer(d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d (5GrwvaEF...), 8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48 (5FHneW46...), 6900000000000000)), topics: [] }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: Event::pallet_treasury(RawEvent::Deposit(1184376000000)), topics: [] }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: Event::frame_system(RawEvent::ExtrinsicSuccess(DispatchInfo { `weight: 460000000,` class: DispatchClass::Normal, pays_fee: Pays::Yes })), topics: [] }]`,
 right: `[EventRecord { phase: Phase::ApplyExtrinsic(0), event: Event::frame_system(RawEvent::ExtrinsicSuccess(DispatchInfo { weight: 0, class: DispatchClass::Mandatory, pays_fee: Pays::Yes })), topics: [] }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: Event::pallet_balances(RawEvent::Transfer(d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d (5GrwvaEF...), 8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48 (5FHneW46...), 6900000000000000)), topics: [] }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: Event::pallet_treasury(RawEvent::Deposit(1184376000000)), topics: [] }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: Event::frame_system(RawEvent::ExtrinsicSuccess(DispatchInfo { weight: 460000000, class: DispatchClass::Normal, pays_fee: Pays::Yes })), topics: [] }]`', bin/node/executor/tests/basic.rs:367:9

weight: 348000000 and weight: 0 sounds to me like the reference needs to be updated to match the new expectation.

At least that's the difference I can spot.

@apopiak
Copy link
Contributor Author

apopiak commented Apr 29, 2020

Yeah, I found that diff as well. I just have no idea how it could be caused by my changes. I'm reluctant to just change the number without knowing why it would change, that's all.

@shawntabrizi
Copy link
Member

bot merge

@ghost ghost merged commit 6dd3ddc into master Apr 30, 2020
@ghost ghost deleted the apopiak-timestamp-weights branch April 30, 2020 11:44
/// dummy `on_initialize` to return the weight used in `on_finalize`.
fn on_initialize() -> Weight {
// weight of `on_finalize`
6_000_000
Copy link
Contributor

Choose a reason for hiding this comment

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

why this is not T::DbWeight::write(1)?

Copy link
Member

Choose a reason for hiding this comment

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

This takes DidUpdate, which is empty at the initialization of the block, populated as the first extrinsic in the block, and then removed again at the finalization of the block.

No value actually gets written to the DB layer. Everything happens within one block in-memory.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants