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

Speed up storage iteration from within the runtime #13479

Merged
merged 7 commits into from
Mar 1, 2023
Merged

Speed up storage iteration from within the runtime #13479

merged 7 commits into from
Mar 1, 2023
+118 −16

Conversation

koute
Copy link
Contributor

@koute koute commented Feb 27, 2023 •
edited
Loading

Followup of #13284 and #13445

This PR makes iterating over storage from within the runtime twice as fast. (It cuts down the time required to ~52% according to the benchmark I've added.)

How does it work?

The way storage iteration is done from within the runtime is through the use of the Storage::next_key host function. What this host function does is pretty simple: given a key it will return the very next key that follows it in lexicographic order. So call it in a loop, and there you go - you're iterating over the storage.

But this is somewhat wasteful since every time this host function is called it will traverse the storage trie from scratch, wasting a lot of time going through nodes it has already went through in the previous iteration. So the trick to speed it up is pretty simple: instantiate an iterator every time this host function is called, stash it somewhere, and then try to reuse it the next time the host function is called if the key that was passed matches the key which the iterator previously returned.

@koute koute added A0-please_review Pull request needs code review. I9-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. B0-silent Changes should not be mentioned in any 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 labels Feb 27, 2023
@koute koute requested review from cheme and a team February 27, 2023 13:06
cheme
cheme approved these changes Feb 27, 2023
View reviewed changes
primitives/state-machine/src/trie_backend.rs Outdated Show resolved Hide resolved
@koute koute force-pushed the master_storage_iterator_faster_next_storage_key branch from acf5040 to c4b85f2 Compare February 28, 2023 07:17
@koute
Copy link
Contributor Author

koute commented Feb 28, 2023

(Sorry for the force push; commits are exactly the same, I just accidentally didn't sign one of the intermediate commits and the only way to rectify that is with a force push.)

bkchr
bkchr approved these changes Feb 28, 2023
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Nice!

I also had this somewhere on my long list! Ty for doing this!

primitives/state-machine/src/trie_backend_essence.rs Outdated Show resolved Hide resolved
@koute
Copy link
Contributor Author

koute commented Mar 1, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit aa31d1b into paritytech:master Mar 1, 2023
gpestana pushed a commit that referenced this pull request Mar 1, 2023
@koute @gpestana
Speed up storage iteration from within the runtime (#13479)
761d7ae 
* Speed up storage iteration from within the runtime

* Move the cached iterator into an `Option`

* Use `RefCell` in no_std

* Simplify the code slightly

* Use `Option::replace`

* Update doc comment for `next_storage_key_slow`
@kiltbot kiltbot mentioned this pull request Apr 3, 2023
Closed
@librelois librelois mentioned this pull request Apr 6, 2023
Closed
ukint-vs pushed a commit to gear-tech/substrate that referenced this pull request Apr 10, 2023
@koute @ukint-vs
Speed up storage iteration from within the runtime (paritytech#13479)
f0a3a77 
* Speed up storage iteration from within the runtime

* Move the cached iterator into an `Option`

* Use `RefCell` in no_std

* Simplify the code slightly

* Use `Option::replace`

* Update doc comment for `next_storage_key_slow`
Neopallium pushed a commit to PolymeshAssociation/substrate that referenced this pull request Jun 5, 2023
@koute @Neopallium
Speed up storage iteration from within the runtime (paritytech#13479)
fb76722 
* Speed up storage iteration from within the runtime

* Move the cached iterator into an `Option`

* Use `RefCell` in no_std

* Simplify the code slightly

* Use `Option::replace`

* Update doc comment for `next_storage_key_slow`
Neopallium pushed a commit to PolymeshAssociation/substrate that referenced this pull request Jun 7, 2023
@koute @Neopallium
Speed up storage iteration from within the runtime (paritytech#13479)
75dd5f1 
* Speed up storage iteration from within the runtime

* Move the cached iterator into an `Option`

* Use `RefCell` in no_std

* Simplify the code slightly

* Use `Option::replace`

* Update doc comment for `next_storage_key_slow`
Neopallium pushed a commit to PolymeshAssociation/substrate that referenced this pull request Jun 7, 2023
@koute @Neopallium
Speed up storage iteration from within the runtime (paritytech#13479)
f15f02b 
* Speed up storage iteration from within the runtime

* Move the cached iterator into an `Option`

* Use `RefCell` in no_std

* Simplify the code slightly

* Use `Option::replace`

* Update doc comment for `next_storage_key_slow`
@kacperzuk-neti kacperzuk-neti mentioned this pull request Jun 23, 2023
Merged
15 tasks
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@koute @nathanwhit
Speed up storage iteration from within the runtime (paritytech#13479)
9c6c1d5 
* Speed up storage iteration from within the runtime

* Move the cached iterator into an `Option`

* Use `RefCell` in no_std

* Simplify the code slightly

* Use `Option::replace`

* Update doc comment for `next_storage_key_slow`
paritytech-processbot bot pushed a commit that referenced this pull request Aug 10, 2023
@gpestana @ggwpez
@davxy @yjhmelody @bkchr @koute @dmitry-markin @lazam @andresilva @pmikolajczyk41 @acatangiu @kianenigma
[NPoS] Implements dynamic number of nominators (#12970)
57d2e53 
* Implements dynamic nominations per nominator

* Adds SnapshotBounds and ElectionSizeTracker

* Changes the ElectionDataProvider interface to receive ElectionBounds as input

* Implements get_npos_voters with ElectionBounds

* Implements get_npos_targets with ElectionBounds

* Adds comments

* tests

* Truncates nomninations that exceed nominations quota; Old tests passing

* Uses DataProviderBounds and ElectionBounds (to continue)

* Finishes conversions - tests passing

* Refactor staking in babe mocks

* Replaces MaxElectableTargets and MaxElectingVoters with ElectionBounds; Adds more tests

* Fixes nits; node compiling

* bechmarks

* removes nomination_quota extrinsic to request the nomination quota

* Lazy quota check, ie. at nominate time only

* remove non-working test (for now)

* tests lazy nominations quota when quota is lower than current number of nominated targets

* Adds runtime API and custom RPC call for clients to query the nominations quota for a given balance

* removes old rpc

* Cosmetic touches

* All mocks working

* Fixes benchmarking mocks

* nits

* more tests

* renames trait methods

* nit

* ".git/.scripts/commands/fmt/fmt.sh"

* Fix V2 PoV benchmarking (#13485)

* Bump default 'additional_trie_layers' to two

The default here only works for extremely small runtimes, which have
no more than 16 storage prefices. This is changed to a "sane" default
of 2, which is save for runtimes with up to 4096 storage prefices (eg StorageValue).

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

* Update tests and test weights

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

* Fix PoV weights

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

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

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

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

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

* Fix sanity check

>0 would also do as a check, but let's try this.

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

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: command-bot <>

* Move BEEFY code to consensus (#13484)

* Move beefy primitives to consensus dir
* Move beefy gadget to client consensus folder
* Rename beefy crates

* chore: move genesis block builder to chain-spec crate. (#13427)

* chore: move genesis block builder to block builder crate.

* add missing file

* chore: move genesis block builder to sc-chain-spec

* Update client/chain-spec/src/genesis.rs

Co-authored-by: Bastian Köcher <[email protected]>

* Update test-utils/runtime/src/genesismap.rs

Co-authored-by: Bastian Köcher <[email protected]>

* Update test-utils/runtime/client/src/lib.rs

* fix warnings

* fix warnings

---------

Co-authored-by: Bastian Köcher <[email protected]>

* Speed up storage iteration from within the runtime (#13479)

* Speed up storage iteration from within the runtime

* Move the cached iterator into an `Option`

* Use `RefCell` in no_std

* Simplify the code slightly

* Use `Option::replace`

* Update doc comment for `next_storage_key_slow`

* Make unbounded channels size warning exact (part 1) (#13490)

* Replace `futures-channel` with `async-channel` in `out_events`

* Apply suggestions from code review

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

* Also print the backtrace of `send()` call

* Switch from `backtrace` crate to `std::backtrace`

* Remove outdated `backtrace` dependency

* Remove `backtrace` from `Cargo.lock`

---------

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

* Removal of Prometheus alerting rules deployment in cloud-infra (#13499)

* sp-consensus: remove unused error variants (#13495)

* Expose `ChargedAmount` (#13488)

* Expose `ChargedAmount`

* Fix imports

* sc-consensus-beefy: fix metrics: use correct names (#13494)


Signed-off-by: acatangiu <[email protected]>

* clippy fix

* removes NominationsQuotaExceeded event

* Update frame/staking/src/lib.rs

Co-authored-by: Ross Bulat <[email protected]>

* adds back the npos_max_iter

* remove duplicate imports added after merge

* fmt

* Adds comment in public struct; Refactors CountBound and SizeCount to struct

* addresses various pr comments

* PR comment reviews

* Fixes on-chain election bounds and related code

* EPM checks the size of the voter list returned by the data provider

* cosmetic changes

* updates e2e tests mock

* Adds more tests for size tracker and refactors code

* Adds back only_iterates_max_2_times_max_allowed_len test

* Refactor

* removes unecessary dependency

* empty commit -- restart all stuck CI jobs

* restarts ci jobs

* Renames ElectionBounds -> Bounds in benchmarking mocks et al

* updates mocks

* Update frame/election-provider-support/src/lib.rs

Co-authored-by: Kian Paimani <[email protected]>

* Update frame/staking/src/pallet/impls.rs

Co-authored-by: Kian Paimani <[email protected]>

* Update frame/election-provider-support/src/lib.rs

Co-authored-by: Kian Paimani <[email protected]>

* Update frame/staking/src/tests.rs

Co-authored-by: Kian Paimani <[email protected]>

* more checks in api_nominations_quota in tests

* Improves docs

* fixes e2e tests

* Uses size_hint rather than mem::size_of in size tracker; Refactor size tracker to own module

* nits from reviews

* Refactors bounds to own module; improves docs

* More tests and docs

* fixes docs

* Fixes benchmarks

* Fixes rust docs

* fixes bags-list remote-ext-tests

* Simplify bound checks in create_snapshot_external

* Adds target size check in get_npos_targets

* ".git/.scripts/commands/fmt/fmt.sh"

* restart ci

* rust doc fixes and cosmetic nits

* rollback upgrade on parity-scale-codec version (unecessary)

* reset cargo lock, no need to update it

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: acatangiu <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Davide Galassi <[email protected]>
Co-authored-by: yjh <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Koute <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
Co-authored-by: Anthony Lazam <[email protected]>
Co-authored-by: André Silva <[email protected]>
Co-authored-by: Piotr Mikołajczyk <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Ross Bulat <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr approved these changes

@cheme cheme cheme approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any 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 I9-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task.
Projects
SDK Node
Status: done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@koute @cheme @bkchr