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

Limit the set of known extrinsics and blocks. #1842

Merged
merged 3 commits into from
Feb 21, 2019

Conversation

twittner
Copy link
Contributor

The set of known extrinsics and blocks per peer currently grows unbounded. This PR attempts to replace those sets with bounded LRU sets.

Open question: What would be a good upper bound? Is 1_000_000 too high?

Part of paritytech/polkadot-sdk#569.

@twittner twittner added the A0-please_review Pull request needs code review. label Feb 21, 2019
/// Maintains the limit of the set by removing the oldest entry if necessary.
pub(crate) fn insert(&mut self, e: T) -> bool {
if self.set.len() == usize::from(self.limit) {
if self.set.contains(&e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about the runtime characteristics of the contains function, but we could always insert optimistically, and then pop_front if needed. This way this will have only one lookup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think insert and contains are both O(1) but your proposal is still more efficient. Also I just learned that inserting existing elements still updates their LRU position which my version does not.

Always inserting first ensures that the element's LRU position is
updated.
@gavofyork gavofyork merged commit 8888fc5 into paritytech:master Feb 21, 2019
MTDK1 pushed a commit to bdevux/substrate that referenced this pull request Apr 12, 2019
* Limit the set of known extrinsics and blocks.

* Update Cargo.lock

* Always insert and pop oldest entry if needed.

Always inserting first ensures that the element's LRU position is
updated.
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.

3 participants