Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Perf

### 2025-11-17

- Avoid temporary allocations when decoding and hashing trie nodes [#5353](https://github.com/lambdaclass/ethrex/pull/5353)

### 2025-11-13

- Use specialized DUP implementation [#5324](https://github.com/lambdaclass/ethrex/pull/5324)
Expand Down
23 changes: 13 additions & 10 deletions crates/common/rlp/structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,19 @@ impl<'a> Decoder<'a> {

/// Returns the next field without decoding it, i.e. the payload bytes including its prefix.
pub fn get_encoded_item(self) -> Result<(Vec<u8>, Self), RLPDecodeError> {
match get_item_with_prefix(self.payload) {
Ok((field, rest)) => {
let updated_self = Self {
payload: rest,
..self
};
Ok((field.to_vec(), updated_self))
}
Err(err) => Err(err),
}
self.get_encoded_item_ref()
.map(|(field, updated_self)| (field.to_vec(), updated_self))
}

/// Returns the next field without decoding it, i.e. the payload bytes including its prefix.
pub fn get_encoded_item_ref(self) -> Result<(&'a [u8], Self), RLPDecodeError> {
get_item_with_prefix(self.payload).map(|(field, rest)| {
let updated_self = Self {
payload: rest,
..self
};
(field, updated_self)
})
}

/// Returns Some(field) if there's some field to decode, otherwise returns None
Expand Down
41 changes: 30 additions & 11 deletions crates/common/trie/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,19 @@ impl NodeRef {
}
}

pub fn memoize_hashes(&self) {
pub fn compute_hash_no_alloc(&self, buf: &mut Vec<u8>) -> &NodeHash {
match self {
NodeRef::Node(node, hash) => hash.get_or_init(|| node.compute_hash_no_alloc(buf)),
NodeRef::Hash(hash) => hash,
}
}

pub fn memoize_hashes(&self, buf: &mut Vec<u8>) {
if let NodeRef::Node(node, hash) = &self
&& hash.get().is_none()
{
node.memoize_hashes();
let _ = hash.set(node.compute_hash());
node.memoize_hashes(buf);
let _ = hash.set(node.compute_hash_no_alloc(buf));
}
}

Expand Down Expand Up @@ -217,7 +224,8 @@ impl From<Arc<Node>> for NodeRef {

impl PartialEq for NodeRef {
fn eq(&self, other: &Self) -> bool {
self.compute_hash() == other.compute_hash()
let mut buf = Vec::new();
self.compute_hash_no_alloc(&mut buf) == other.compute_hash_no_alloc(&mut buf)
}
}

Expand Down Expand Up @@ -358,24 +366,35 @@ impl Node {

/// Computes the node's hash
pub fn compute_hash(&self) -> NodeHash {
self.memoize_hashes();
let mut buf = Vec::new();
self.memoize_hashes(&mut buf);
match self {
Node::Branch(n) => n.compute_hash_no_alloc(&mut buf),
Node::Extension(n) => n.compute_hash_no_alloc(&mut buf),
Node::Leaf(n) => n.compute_hash_no_alloc(&mut buf),
}
}

/// Computes the node's hash
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

The doc comment is identical to the one for compute_hash above. Consider clarifying that this function uses a provided buffer to avoid allocations, e.g., "Computes the node's hash using the provided buffer to avoid allocations."

Suggested change
/// Computes the node's hash
/// Computes the node's hash using the provided buffer to avoid allocations

Copilot uses AI. Check for mistakes.
pub fn compute_hash_no_alloc(&self, buf: &mut Vec<u8>) -> NodeHash {
self.memoize_hashes(buf);
match self {
Node::Branch(n) => n.compute_hash(),
Node::Extension(n) => n.compute_hash(),
Node::Leaf(n) => n.compute_hash(),
Node::Branch(n) => n.compute_hash_no_alloc(buf),
Node::Extension(n) => n.compute_hash_no_alloc(buf),
Node::Leaf(n) => n.compute_hash_no_alloc(buf),
}
}

/// Recursively memoizes the hashes of all nodes of the subtrie that has
/// `self` as root (post-order traversal)
pub fn memoize_hashes(&self) {
pub fn memoize_hashes(&self, buf: &mut Vec<u8>) {
match self {
Node::Branch(n) => {
for child in &n.choices {
child.memoize_hashes();
child.memoize_hashes(buf);
}
}
Node::Extension(n) => n.child.memoize_hashes(),
Node::Extension(n) => n.child.memoize_hashes(buf),
_ => {}
}
}
Expand Down
11 changes: 10 additions & 1 deletion crates/common/trie/node/branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,16 @@ impl BranchNode {

/// Computes the node's hash
pub fn compute_hash(&self) -> NodeHash {
NodeHash::from_encoded(&self.encode_to_vec())
self.compute_hash_no_alloc(&mut vec![])
}

/// Computes the node's hash, using the provided buffer
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

The doc comment is identical to the one for compute_hash above. Consider clarifying that this function uses a provided buffer to avoid allocations, e.g., "Computes the node's hash using the provided buffer to avoid allocations."

Suggested change
/// Computes the node's hash, using the provided buffer
/// Computes the node's hash using the provided buffer to avoid allocations.
/// This method reuses the given buffer to minimize heap allocations when encoding the node.

Copilot uses AI. Check for mistakes.
pub fn compute_hash_no_alloc(&self, buf: &mut Vec<u8>) -> NodeHash {
buf.clear();
self.encode(buf);
let hash = NodeHash::from_encoded(buf);
buf.clear();
hash
}

/// Traverses own subtrie until reaching the node containing `path`
Expand Down
11 changes: 10 additions & 1 deletion crates/common/trie/node/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,16 @@ impl ExtensionNode {

/// Computes the node's hash
pub fn compute_hash(&self) -> NodeHash {
NodeHash::from_encoded(&self.encode_to_vec())
self.compute_hash_no_alloc(&mut vec![])
}

/// Computes the node's hash, using the provided buffer
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

The doc comment is identical to the one for compute_hash above. Consider clarifying that this function uses a provided buffer to avoid allocations, e.g., "Computes the node's hash using the provided buffer to avoid allocations."

Suggested change
/// Computes the node's hash, using the provided buffer
/// Computes the node's hash using the provided buffer to avoid allocations.

Copilot uses AI. Check for mistakes.
pub fn compute_hash_no_alloc(&self, buf: &mut Vec<u8>) -> NodeHash {
buf.clear();
self.encode(buf);
let hash = NodeHash::from_encoded(buf);
buf.clear();
hash
}

/// Traverses own subtrie until reaching the node containing `path`
Expand Down
11 changes: 10 additions & 1 deletion crates/common/trie/node/leaf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,16 @@ impl LeafNode {

/// Computes the node's hash
pub fn compute_hash(&self) -> NodeHash {
NodeHash::from_encoded(&self.encode_to_vec())
self.compute_hash_no_alloc(&mut vec![])
}

/// Computes the node's hash, using the provided buffer
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

The doc comment is identical to the one for compute_hash above. Consider clarifying that this function uses a provided buffer to avoid allocations, e.g., "Computes the node's hash using the provided buffer to avoid allocations."

Suggested change
/// Computes the node's hash, using the provided buffer
/// Computes the node's hash using the provided buffer to avoid allocations.

Copilot uses AI. Check for mistakes.
pub fn compute_hash_no_alloc(&self, buf: &mut Vec<u8>) -> NodeHash {
buf.clear();
self.encode(buf);
let hash = NodeHash::from_encoded(buf);
buf.clear();
hash
}

/// Encodes the node and appends it to `node_path` if the encoded node is 32 or more bytes long
Expand Down
90 changes: 50 additions & 40 deletions crates/common/trie/rlp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,58 +86,68 @@ impl RLPEncode for Node {

impl RLPDecode for Node {
fn decode_unfinished(rlp: &[u8]) -> Result<(Self, &[u8]), RLPDecodeError> {
let mut rlp_items = vec![];
let mut rlp_items_len = 0;
let mut rlp_items: [Option<&[u8]>; 17] = Default::default();
let mut decoder = Decoder::new(rlp)?;
let mut item;
// Get encoded fields

// Check if we reached the end or if we decoded more items than the ones we need
while !decoder.is_done() && rlp_items.len() <= 17 {
(item, decoder) = decoder.get_encoded_item()?;
rlp_items.push(item);
while !decoder.is_done() && rlp_items_len < 17 {
(item, decoder) = decoder.get_encoded_item_ref()?;
rlp_items[rlp_items_len] = Some(item);
rlp_items_len += 1;
}
if !decoder.is_done() {
return Err(RLPDecodeError::Custom(
"Invalid arg count for Node, expected 2 or 17, got more than 17".to_string(),
));
}
// Deserialize into node depending on the available fields
Ok((
match rlp_items.len() {
// Leaf or Extension Node
2 => {
let (path, _) = decode_bytes(&rlp_items[0])?;
let path = Nibbles::decode_compact(path);
if path.is_leaf() {
// Decode as Leaf
let (value, _) = decode_bytes(&rlp_items[1])?;
LeafNode {
partial: path,
value: value.to_vec(),
}
.into()
} else {
// Decode as Extension
ExtensionNode {
prefix: path,
child: decode_child(&rlp_items[1]).into(),
}
.into()
}
}
// Branch Node
17 => {
let choices = array::from_fn(|i| decode_child(&rlp_items[i]).into());
let (value, _) = decode_bytes(&rlp_items[16])?;
BranchNode {
choices,
let node = match rlp_items_len {
// Leaf or Extension Node
2 => {
let (path, _) = decode_bytes(rlp_items[0].expect("we already checked the length"))?;
let path = Nibbles::decode_compact(path);
if path.is_leaf() {
// Decode as Leaf
let (value, _) =
decode_bytes(rlp_items[1].expect("we already checked the length"))?;
LeafNode {
partial: path,
value: value.to_vec(),
}
.into()
} else {
// Decode as Extension
ExtensionNode {
prefix: path,
child: decode_child(rlp_items[1].expect("we already checked the length"))
.into(),
}
.into()
}
n => {
return Err(RLPDecodeError::Custom(format!(
"Invalid arg count for Node, expected 2 or 17, got {n}"
)));
}
// Branch Node
17 => {
let choices = array::from_fn(|i| {
decode_child(rlp_items[i].expect("we already checked the length")).into()
});
let (value, _) =
decode_bytes(rlp_items[16].expect("we already checked the length"))?;
BranchNode {
choices,
value: value.to_vec(),
}
},
decoder.finish()?,
))
.into()
}
n => {
return Err(RLPDecodeError::Custom(format!(
"Invalid arg count for Node, expected 2 or 17, got {n}"
)));
}
};
Ok((node, decoder.finish()?))
}
}

Expand Down
4 changes: 3 additions & 1 deletion crates/common/trie/trie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,9 @@ impl Trie {
/// Returns keccak(RLP_NULL) if the trie is empty
pub fn hash_no_commit(&self) -> H256 {
if self.root.is_valid() {
self.root.compute_hash().finalize()
// 512 is the maximum size of an encoded node
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

The comment states "512 is the maximum size of an encoded node", but this may not be accurate for all cases. A branch node with 16 children can exceed this size. While Vec will automatically grow, the comment should be updated to reflect that 512 is an estimated typical size rather than a strict maximum.

Suggested change
// 512 is the maximum size of an encoded node
// 512 is an estimated typical size for an encoded node; some nodes (e.g., branch nodes with many children) may exceed this size

Copilot uses AI. Check for mistakes.
let mut buf = Vec::with_capacity(512);
self.root.compute_hash_no_alloc(&mut buf).finalize()
} else {
*EMPTY_TRIE_HASH
}
Expand Down
19 changes: 13 additions & 6 deletions crates/common/trie/trie_sorted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ fn add_center_to_parent_and_write_queue(
) -> Result<(), TrieGenerationError> {
debug!("{:x?}", center_side.path);
debug!("{:x?}", parent_element.path);
let mut nodehash_buffer = Vec::with_capacity(512);
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

[nitpick] The initial capacity of 512 bytes may be insufficient for some branch nodes. A branch node can have 16 children (each encoded as up to 33 bytes) plus a value field, potentially exceeding 512 bytes with RLP overhead. While Vec will automatically grow, this may cause reallocations. Consider using a larger initial capacity (e.g., 600-700 bytes) or document this trade-off.

Suggested change
let mut nodehash_buffer = Vec::with_capacity(512);
// Increased initial capacity to 700 bytes to avoid reallocations for large branch nodes.
let mut nodehash_buffer = Vec::with_capacity(700);

Copilot uses AI. Check for mistakes.
let mut path = center_side.path.clone();
path.skip_prefix(&parent_element.path);
let index = path
Expand All @@ -96,7 +97,7 @@ fn add_center_to_parent_and_write_queue(
if path.is_empty() {
(top_path, node.clone().into())
} else {
let hash = node.compute_hash();
let hash = node.compute_hash_no_alloc(&mut nodehash_buffer);
nodes_to_write.push((center_side.path.clone(), node.clone().into()));
(
top_path,
Expand All @@ -117,7 +118,8 @@ fn add_center_to_parent_and_write_queue(
.into(),
),
};
parent_element.element.choices[index as usize] = node.compute_hash().into();
parent_element.element.choices[index as usize] =
node.compute_hash_no_alloc(&mut nodehash_buffer).into();
debug!(
"branch {:x?}",
parent_element
Expand Down Expand Up @@ -167,6 +169,7 @@ where
let mut left_side = StackElement::default();
let mut center_side: CenterSide = CenterSide::from_value(initial_value);
let mut right_side_opt: Option<(H256, Vec<u8>)> = data_iter.next();
let mut nodehash_buffer = Vec::with_capacity(512);
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

[nitpick] The initial capacity of 512 bytes may be insufficient for some branch nodes. A branch node can have 16 children (each encoded as up to 33 bytes) plus a value field, potentially exceeding 512 bytes with RLP overhead. While Vec will automatically grow, this may cause reallocations. Consider using a larger initial capacity (e.g., 600-700 bytes) or document this trade-off.

Suggested change
let mut nodehash_buffer = Vec::with_capacity(512);
// Increased initial capacity to 700 bytes to avoid reallocations for large branch nodes (16 children * 33 bytes + value + RLP overhead)
let mut nodehash_buffer = Vec::with_capacity(700);

Copilot uses AI. Check for mistakes.

while let Some(right_side) = right_side_opt {
if nodes_to_write.len() as u64 > SIZE_TO_WRITE_DB {
Expand Down Expand Up @@ -259,22 +262,26 @@ where
.last()
.expect("we just inserted")
.1
.compute_hash()
.compute_hash_no_alloc(&mut nodehash_buffer)
.finalize()
}
Node::Extension(extension_node) => {
extension_node.prefix.prepend(index as u8);
// This next works because this target path is always length of 1 element,
// and we're just removing that one element
target_path.next();
extension_node.compute_hash().finalize()
extension_node
.compute_hash_no_alloc(&mut nodehash_buffer)
.finalize()
}
Node::Leaf(leaf_node) => {
leaf_node.partial.prepend(index as u8);
// This next works because this target path is always length of 1 element,
// and we're just removing that one element
target_path.next();
leaf_node.compute_hash().finalize()
leaf_node
.compute_hash_no_alloc(&mut nodehash_buffer)
.finalize()
}
}
} else {
Expand All @@ -284,7 +291,7 @@ where
.last()
.expect("we just inserted")
.1
.compute_hash()
.compute_hash_no_alloc(&mut nodehash_buffer)
.finalize()
};

Expand Down
Loading