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

Use storage::append in the implementation of the storage types #5889

Merged
merged 7 commits into from
May 5, 2020

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented May 5, 2020

This pr moves all the append_* functions to use the new storage::append functionality. This should improve the performance of all these calls. Besides that the host implementation is changed to use the EncodeAppend from parity-scale-codec instead of copying it.

The implementation comes with some "disadvantages":

  • If the storage item is invalid, append will overwrite it and also ignores any default value set in the runtime. Sadly there isn't a better solution for this, but we also should teach people that vectors etc should not get any special default values.
  • It now can only append one item at a time. Up to now, there was only one call to append that tried to append more items.

@@ -526,47 +526,47 @@ mod test_append_and_len {
#[test]
fn append_works() {
Copy link
Contributor

Choose a reason for hiding this comment

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

kill + append
overwrite + append

would be nice additions to this test

Copy link
Member Author

Choose a reason for hiding this comment

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

overwrite + append
Is done below in append_overwrites_invalid_data

Copy link
Member Author

Choose a reason for hiding this comment

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

Added kill + append

@NikVolf
Copy link
Contributor

NikVolf commented May 5, 2020

/bench import/full

@parity-benchapp
Copy link

parity-benchapp bot commented May 5, 2020

Finished benchmark for branch: bkchr-storage-append-improvements

Benchmark: Import Benchmark (Full block with wasm, for weights validation)

Master: 1333.89 ms
Branch: 1326.94 ms
Change: -0.52%

@paritytech paritytech deleted a comment from parity-benchapp bot May 5, 2020
@paritytech paritytech deleted a comment from parity-benchapp bot May 5, 2020
@gavofyork
Copy link
Member

polkadot not building?

@bkchr
Copy link
Member Author

bkchr commented May 5, 2020

Yeah, already working on a companion.

@bkchr bkchr merged commit 2bb06f1 into master May 5, 2020
@bkchr bkchr deleted the bkchr-storage-append-improvements branch May 5, 2020 13:09
lamafab pushed a commit to lamafab/substrate that referenced this pull request Jun 16, 2020
* Companion pr for Substrate paritytech#5889

* Update Substrate ref
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants