From 45f7a77850f08d0f493040c1eccbf03a4061a047 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 27 Sep 2024 16:57:08 +0200 Subject: [PATCH] perf: improve IntegerList API to avoid allocations --- Cargo.lock | 1 - crates/primitives-traits/src/integer_list.rs | 46 ++++++------------- .../src/segments/user/account_history.rs | 6 +-- .../prune/prune/src/segments/user/history.rs | 10 ++-- .../src/segments/user/storage_history.rs | 6 +-- .../src/stages/index_account_history.rs | 2 +- .../src/stages/index_storage_history.rs | 2 +- crates/stages/stages/src/stages/utils.rs | 6 +-- .../storage/db-api/src/models/integer_list.rs | 1 + .../storage/db/src/implementation/mdbx/mod.rs | 6 +-- .../db/src/tables/codecs/fuzz/inputs.rs | 13 +++--- .../src/providers/database/provider.rs | 2 +- examples/custom-inspector/Cargo.toml | 1 - 13 files changed, 39 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7e302ec0471e3..3b9c6624bec24 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2762,7 +2762,6 @@ dependencies = [ "futures-util", "reth", "reth-node-ethereum", - "reth-rpc-types", ] [[package]] diff --git a/crates/primitives-traits/src/integer_list.rs b/crates/primitives-traits/src/integer_list.rs index 767fb3ec30a1c..5777a430d5cc9 100644 --- a/crates/primitives-traits/src/integer_list.rs +++ b/crates/primitives-traits/src/integer_list.rs @@ -16,22 +16,19 @@ pub struct IntegerList(pub RoaringTreemap); impl fmt::Debug for IntegerList { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let vec: Vec = self.0.iter().collect(); - write!(f, "IntegerList {vec:?}") + f.write_str("IntegerList")?; + f.debug_list().entries(self.0.iter()).finish() } } impl IntegerList { /// Creates an `IntegerList` from a list of integers. /// - /// # Returns - /// /// Returns an error if the list is empty or not pre-sorted. - pub fn new>(list: T) -> Result { - Ok(Self( - RoaringTreemap::from_sorted_iter(list.as_ref().iter().copied()) - .map_err(|_| RoaringBitmapError::InvalidInput)?, - )) + pub fn new(list: impl IntoIterator) -> Result { + RoaringTreemap::from_sorted_iter(list) + .map(Self) + .map_err(|_| RoaringBitmapError::InvalidInput) } // Creates an IntegerList from a pre-sorted list of integers. @@ -39,11 +36,10 @@ impl IntegerList { /// # Panics /// /// Panics if the list is empty or not pre-sorted. - pub fn new_pre_sorted>(list: T) -> Self { - Self( - RoaringTreemap::from_sorted_iter(list.as_ref().iter().copied()) - .expect("IntegerList must be pre-sorted and non-empty"), - ) + #[inline] + #[track_caller] + pub fn new_pre_sorted(list: impl IntoIterator) -> Self { + Self::new(list).expect("IntegerList must be pre-sorted and non-empty") } /// Serializes a [`IntegerList`] into a sequence of bytes. @@ -67,28 +63,13 @@ impl IntegerList { } } -macro_rules! impl_uint { - ($($w:tt),+) => { - $( - impl From> for IntegerList { - fn from(v: Vec<$w>) -> Self { - Self::new_pre_sorted(v.iter().map(|v| *v as u64).collect::>()) - } - } - )+ - }; -} - -impl_uint!(usize, u64, u32, u8, u16); - impl Serialize for IntegerList { fn serialize(&self, serializer: S) -> Result where S: Serializer, { - let vec = self.0.iter().collect::>(); let mut seq = serializer.serialize_seq(Some(self.len() as usize))?; - for e in vec { + for e in &self.0 { seq.serialize_element(&e)?; } seq.end() @@ -107,11 +88,10 @@ impl<'de> Visitor<'de> for IntegerListVisitor { where E: SeqAccess<'de>, { - let mut list = Vec::new(); + let mut list = Vec::with_capacity(seq.size_hint().unwrap_or(0).min(1024)); while let Some(item) = seq.next_element()? { list.push(item); } - IntegerList::new(list).map_err(|_| serde::de::Error::invalid_value(Unexpected::Seq, &self)) } } @@ -132,7 +112,7 @@ use arbitrary::{Arbitrary, Unstructured}; impl<'a> Arbitrary<'a> for IntegerList { fn arbitrary(u: &mut Unstructured<'a>) -> Result { let mut nums: Vec = Vec::arbitrary(u)?; - nums.sort(); + nums.sort_unstable(); Self::new(nums).map_err(|_| arbitrary::Error::IncorrectFormat) } } diff --git a/crates/prune/prune/src/segments/user/account_history.rs b/crates/prune/prune/src/segments/user/account_history.rs index 016d9a22fba2f..01f8c0850a187 100644 --- a/crates/prune/prune/src/segments/user/account_history.rs +++ b/crates/prune/prune/src/segments/user/account_history.rs @@ -275,10 +275,8 @@ mod tests { .iter() .filter(|(key, _)| key.highest_block_number > last_pruned_block_number) .map(|(key, blocks)| { - let new_blocks = blocks - .iter() - .skip_while(|block| *block <= last_pruned_block_number) - .collect::>(); + let new_blocks = + blocks.iter().skip_while(|block| *block <= last_pruned_block_number); (key.clone(), BlockNumberList::new_pre_sorted(new_blocks)) }) .collect::>(); diff --git a/crates/prune/prune/src/segments/user/history.rs b/crates/prune/prune/src/segments/user/history.rs index e27884a927807..675f6d79550b6 100644 --- a/crates/prune/prune/src/segments/user/history.rs +++ b/crates/prune/prune/src/segments/user/history.rs @@ -101,18 +101,18 @@ where // contain the target block number, as it's in this shard. else { let blocks = raw_blocks.value()?; - let higher_blocks = - blocks.iter().skip_while(|block| *block <= to_block).collect::>(); + let higher_blocks = || blocks.iter().skip_while(|block| *block <= to_block); + let n_higher_blocks = higher_blocks().count(); // If there were blocks less than or equal to the target one // (so the shard has changed), update the shard. - if blocks.len() as usize == higher_blocks.len() { + if blocks.len() as usize == n_higher_blocks { return Ok(PruneShardOutcome::Unchanged); } // If there will be no more blocks in the shard after pruning blocks below target // block, we need to remove it, as empty shards are not allowed. - if higher_blocks.is_empty() { + if n_higher_blocks == 0 { if key.as_ref().highest_block_number == u64::MAX { let prev_row = cursor .prev()? @@ -151,7 +151,7 @@ where } else { cursor.upsert( RawKey::new(key), - RawValue::new(BlockNumberList::new_pre_sorted(higher_blocks)), + RawValue::new(BlockNumberList::new_pre_sorted(higher_blocks())), )?; Ok(PruneShardOutcome::Updated) } diff --git a/crates/prune/prune/src/segments/user/storage_history.rs b/crates/prune/prune/src/segments/user/storage_history.rs index 5291d822cefaf..315ad750a8b77 100644 --- a/crates/prune/prune/src/segments/user/storage_history.rs +++ b/crates/prune/prune/src/segments/user/storage_history.rs @@ -281,10 +281,8 @@ mod tests { .iter() .filter(|(key, _)| key.sharded_key.highest_block_number > last_pruned_block_number) .map(|(key, blocks)| { - let new_blocks = blocks - .iter() - .skip_while(|block| *block <= last_pruned_block_number) - .collect::>(); + let new_blocks = + blocks.iter().skip_while(|block| *block <= last_pruned_block_number); (key.clone(), BlockNumberList::new_pre_sorted(new_blocks)) }) .collect::>(); diff --git a/crates/stages/stages/src/stages/index_account_history.rs b/crates/stages/stages/src/stages/index_account_history.rs index e0fcde2b194f5..39d50242aff7a 100644 --- a/crates/stages/stages/src/stages/index_account_history.rs +++ b/crates/stages/stages/src/stages/index_account_history.rs @@ -182,7 +182,7 @@ mod tests { } fn list(list: &[u64]) -> BlockNumberList { - BlockNumberList::new(list).unwrap() + BlockNumberList::new(list.iter().copied()).unwrap() } fn cast( diff --git a/crates/stages/stages/src/stages/index_storage_history.rs b/crates/stages/stages/src/stages/index_storage_history.rs index 4af2cb3efea2e..efeec4a3a217d 100644 --- a/crates/stages/stages/src/stages/index_storage_history.rs +++ b/crates/stages/stages/src/stages/index_storage_history.rs @@ -197,7 +197,7 @@ mod tests { } fn list(list: &[u64]) -> BlockNumberList { - BlockNumberList::new(list).unwrap() + BlockNumberList::new(list.iter().copied()).unwrap() } fn cast( diff --git a/crates/stages/stages/src/stages/utils.rs b/crates/stages/stages/src/stages/utils.rs index 7cdab4ff24489..caf039faca108 100644 --- a/crates/stages/stages/src/stages/utils.rs +++ b/crates/stages/stages/src/stages/utils.rs @@ -54,11 +54,11 @@ where let mut cache: HashMap> = HashMap::default(); let mut collect = |cache: &HashMap>| { - for (key, indice_list) in cache { - let last = indice_list.last().expect("qed"); + for (key, indices) in cache { + let last = indices.last().expect("qed"); collector.insert( sharded_key_factory(*key, *last), - BlockNumberList::new_pre_sorted(indice_list), + BlockNumberList::new_pre_sorted(indices.iter().copied()), )?; } Ok::<(), StageError>(()) diff --git a/crates/storage/db-api/src/models/integer_list.rs b/crates/storage/db-api/src/models/integer_list.rs index f47605bf88b52..6d28b5496a1bb 100644 --- a/crates/storage/db-api/src/models/integer_list.rs +++ b/crates/storage/db-api/src/models/integer_list.rs @@ -12,6 +12,7 @@ impl Compress for IntegerList { fn compress(self) -> Self::Compressed { self.to_bytes() } + fn compress_to_buf>(self, buf: &mut B) { self.to_mut_bytes(buf) } diff --git a/crates/storage/db/src/implementation/mdbx/mod.rs b/crates/storage/db/src/implementation/mdbx/mod.rs index 8b4a136c300d8..1deb86ba614f7 100644 --- a/crates/storage/db/src/implementation/mdbx/mod.rs +++ b/crates/storage/db/src/implementation/mdbx/mod.rs @@ -1319,7 +1319,7 @@ mod tests { for i in 1..5 { let key = ShardedKey::new(real_key, i * 100); - let list: IntegerList = vec![i * 100u64].into(); + let list = IntegerList::new_pre_sorted([i * 100u64]); db.update(|tx| tx.put::(key.clone(), list.clone()).expect("")) .unwrap(); @@ -1340,7 +1340,7 @@ mod tests { .expect("should be able to retrieve it."); assert_eq!(ShardedKey::new(real_key, 200), key); - let list200: IntegerList = vec![200u64].into(); + let list200 = IntegerList::new_pre_sorted([200u64]); assert_eq!(list200, list); } // Seek greatest index @@ -1357,7 +1357,7 @@ mod tests { .expect("should be able to retrieve it."); assert_eq!(ShardedKey::new(real_key, 400), key); - let list400: IntegerList = vec![400u64].into(); + let list400 = IntegerList::new_pre_sorted([400u64]); assert_eq!(list400, list); } } diff --git a/crates/storage/db/src/tables/codecs/fuzz/inputs.rs b/crates/storage/db/src/tables/codecs/fuzz/inputs.rs index 2c944e158eb18..23282746fa798 100644 --- a/crates/storage/db/src/tables/codecs/fuzz/inputs.rs +++ b/crates/storage/db/src/tables/codecs/fuzz/inputs.rs @@ -10,12 +10,13 @@ pub struct IntegerListInput(pub Vec); impl From for IntegerList { fn from(list: IntegerListInput) -> Self { let mut v = list.0; - // Empty lists are not supported by `IntegerList`, so we want to skip these cases. - if v.is_empty() { - return vec![1u64].into() - } - v.sort(); - v.into() + let list = if v.is_empty() { + &[1u64][..] + } else { + v.sort_unstable(); + &v + }; + IntegerList::new_pre_sorted(list.iter().copied()) } } diff --git a/crates/storage/provider/src/providers/database/provider.rs b/crates/storage/provider/src/providers/database/provider.rs index 6e026f5c910ab..7159720bf3715 100644 --- a/crates/storage/provider/src/providers/database/provider.rs +++ b/crates/storage/provider/src/providers/database/provider.rs @@ -1356,7 +1356,7 @@ impl DatabaseProvider { }; self.tx.put::( sharded_key_factory(partial_key, highest_block_number), - BlockNumberList::new_pre_sorted(list), + BlockNumberList::new_pre_sorted(list.iter().copied()), )?; } } diff --git a/examples/custom-inspector/Cargo.toml b/examples/custom-inspector/Cargo.toml index a94980951627e..18629556c42fe 100644 --- a/examples/custom-inspector/Cargo.toml +++ b/examples/custom-inspector/Cargo.toml @@ -8,7 +8,6 @@ license.workspace = true [dependencies] reth.workspace = true reth-node-ethereum.workspace = true -reth-rpc-types.workspace = true alloy-rpc-types.workspace = true clap = { workspace = true, features = ["derive"] } futures-util.workspace = true