-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf(primitives): avoid cloning receipts when calculating the root #8350
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,6 @@ use crate::{ | |
B256, U256, | ||
}; | ||
use alloy_rlp::Encodable; | ||
use bytes::BufMut; | ||
use itertools::Itertools; | ||
|
||
/// Adjust the index of an item for rlp encoding. | ||
|
@@ -30,18 +29,16 @@ pub fn ordered_trie_root<T: Encodable>(items: &[T]) -> B256 { | |
/// Compute a trie root of the collection of items with a custom encoder. | ||
pub fn ordered_trie_root_with_encoder<T, F>(items: &[T], mut encode: F) -> B256 | ||
where | ||
F: FnMut(&T, &mut dyn BufMut), | ||
F: FnMut(&T, &mut Vec<u8>), | ||
{ | ||
let mut index_buffer = Vec::new(); | ||
let mut value_buffer = Vec::new(); | ||
|
||
let mut hb = HashBuilder::default(); | ||
let items_len = items.len(); | ||
for i in 0..items_len { | ||
let index = adjust_index_for_rlp(i, items_len); | ||
|
||
index_buffer.clear(); | ||
index.encode(&mut index_buffer); | ||
let index_buffer = alloy_rlp::encode_fixed_size(&index); | ||
|
||
value_buffer.clear(); | ||
encode(&items[index], &mut value_buffer); | ||
|
@@ -104,20 +101,25 @@ pub fn calculate_receipt_root_optimism( | |
ordered_trie_root_with_encoder(receipts, |r, buf| r.encode_inner(buf, false)) | ||
} | ||
|
||
/// Calculates the receipt root for a header. | ||
pub fn calculate_receipt_root_ref(receipts: &[ReceiptWithBloomRef<'_>]) -> B256 { | ||
ordered_trie_root_with_encoder(receipts, |r, buf| r.encode_inner(buf, false)) | ||
} | ||
|
||
/// Calculates the receipt root for a header for the reference type of [Receipt]. | ||
/// | ||
/// NOTE: Prefer [calculate_receipt_root] if you have log blooms memoized. | ||
pub fn calculate_receipt_root_ref(receipts: &[&Receipt]) -> B256 { | ||
/// NOTE: Prefer [`calculate_receipt_root`] if you have log blooms memoized. | ||
pub fn calculate_receipt_root_no_memo(receipts: &[&Receipt]) -> B256 { | ||
ordered_trie_root_with_encoder(receipts, |r, buf| { | ||
ReceiptWithBloomRef::from(*r).encode_inner(buf, false) | ||
}) | ||
} | ||
|
||
/// Calculates the receipt root for a header for the reference type of [Receipt]. | ||
/// | ||
/// NOTE: Prefer [calculate_receipt_root] if you have log blooms memoized. | ||
/// NOTE: Prefer [`calculate_receipt_root_optimism`] if you have log blooms memoized. | ||
#[cfg(feature = "optimism")] | ||
pub fn calculate_receipt_root_ref_optimism( | ||
pub fn calculate_receipt_root_no_memo_optimism( | ||
Comment on lines
+120
to
+122
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not to self to move this shortly |
||
receipts: &[&Receipt], | ||
chain_spec: &crate::ChainSpec, | ||
timestamp: u64, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,6 +56,12 @@ impl Receipt { | |
pub fn with_bloom(self) -> ReceiptWithBloom { | ||
self.into() | ||
} | ||
|
||
/// Calculates the bloom filter for the receipt and returns the [ReceiptWithBloomRef] container | ||
/// type. | ||
pub fn with_bloom_ref(&self) -> ReceiptWithBloomRef<'_> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as_bloom_ref ? maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thats what the type is called, it would also make more sense to highlight that this is slow and not have Into impls |
||
self.into() | ||
} | ||
} | ||
|
||
/// A collection of receipts organized as a two-dimensional vector. | ||
|
@@ -98,7 +104,7 @@ impl Receipts { | |
|
||
/// Retrieves the receipt root for all recorded receipts from index. | ||
pub fn root_slow(&self, index: usize) -> Option<B256> { | ||
Some(crate::proofs::calculate_receipt_root_ref( | ||
Some(crate::proofs::calculate_receipt_root_no_memo( | ||
&self.receipt_vec[index].iter().map(Option::as_ref).collect::<Option<Vec<_>>>()?, | ||
)) | ||
} | ||
|
@@ -111,7 +117,7 @@ impl Receipts { | |
chain_spec: &crate::ChainSpec, | ||
timestamp: u64, | ||
) -> Option<B256> { | ||
Some(crate::proofs::calculate_receipt_root_ref_optimism( | ||
Some(crate::proofs::calculate_receipt_root_no_memo_optimism( | ||
&self.receipt_vec[index].iter().map(Option::as_ref).collect::<Option<Vec<_>>>()?, | ||
chain_spec, | ||
timestamp, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sigh, would be nice to have one method for both cases, unsure if it's possible though