chore(tree): replace LRU with FIFO for block tracking (#14587)

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
This commit is contained in:
Naman Garg
2025-02-20 06:43:28 -08:00
committed by GitHub
parent 2a8f5b883b
commit 0500069772

View File

@ -2,8 +2,7 @@ use crate::tree::metrics::BlockBufferMetrics;
use alloy_consensus::BlockHeader; use alloy_consensus::BlockHeader;
use alloy_primitives::{BlockHash, BlockNumber}; use alloy_primitives::{BlockHash, BlockNumber};
use reth_primitives_traits::{Block, RecoveredBlock}; use reth_primitives_traits::{Block, RecoveredBlock};
use schnellru::{ByLength, LruMap}; use std::collections::{BTreeMap, HashMap, HashSet, VecDeque};
use std::collections::{BTreeMap, HashMap, HashSet};
/// Contains the tree of pending blocks that cannot be executed due to missing parent. /// Contains the tree of pending blocks that cannot be executed due to missing parent.
/// It allows to store unconnected blocks for potential future inclusion. /// It allows to store unconnected blocks for potential future inclusion.
@ -27,11 +26,11 @@ pub struct BlockBuffer<B: Block> {
/// `BTreeMap` tracking the earliest blocks by block number. /// `BTreeMap` tracking the earliest blocks by block number.
/// Used for removal of old blocks that precede finalization. /// Used for removal of old blocks that precede finalization.
pub(crate) earliest_blocks: BTreeMap<BlockNumber, HashSet<BlockHash>>, pub(crate) earliest_blocks: BTreeMap<BlockNumber, HashSet<BlockHash>>,
/// LRU used for tracing oldest inserted blocks that are going to be /// FIFO queue tracking block insertion order for eviction.
/// first in line for evicting if `max_blocks` limit is hit. /// When the buffer reaches its capacity limit, the oldest block is evicted first.
/// pub(crate) block_queue: VecDeque<BlockHash>,
/// Used as counter of amount of blocks inside buffer. /// Maximum number of blocks that can be stored in the buffer
pub(crate) lru: LruMap<BlockHash, ()>, pub(crate) max_blocks: usize,
/// Various metrics for the block buffer. /// Various metrics for the block buffer.
pub(crate) metrics: BlockBufferMetrics, pub(crate) metrics: BlockBufferMetrics,
} }
@ -43,7 +42,8 @@ impl<B: Block> BlockBuffer<B> {
blocks: Default::default(), blocks: Default::default(),
parent_to_child: Default::default(), parent_to_child: Default::default(),
earliest_blocks: Default::default(), earliest_blocks: Default::default(),
lru: LruMap::new(ByLength::new(limit)), block_queue: VecDeque::default(),
max_blocks: limit as usize,
metrics: Default::default(), metrics: Default::default(),
} }
} }
@ -70,28 +70,19 @@ impl<B: Block> BlockBuffer<B> {
self.earliest_blocks.entry(block.number()).or_default().insert(hash); self.earliest_blocks.entry(block.number()).or_default().insert(hash);
self.blocks.insert(hash, block); self.blocks.insert(hash, block);
if let Some(evicted_hash) = self.insert_hash_and_get_evicted(hash) { // Add block to FIFO queue and handle eviction if needed
// evict the block if limit is hit if self.block_queue.len() >= self.max_blocks {
if let Some(evicted_block) = self.remove_block(&evicted_hash) { // Evict oldest block if limit is hit
// evict the block if limit is hit if let Some(evicted_hash) = self.block_queue.pop_front() {
self.remove_from_parent(evicted_block.parent_hash(), &evicted_hash); if let Some(evicted_block) = self.remove_block(&evicted_hash) {
self.remove_from_parent(evicted_block.parent_hash(), &evicted_hash);
}
} }
} }
self.block_queue.push_back(hash);
self.metrics.blocks.set(self.blocks.len() as f64); self.metrics.blocks.set(self.blocks.len() as f64);
} }
/// Inserts the hash and returns the oldest evicted hash if any.
fn insert_hash_and_get_evicted(&mut self, entry: BlockHash) -> Option<BlockHash> {
let new = self.lru.peek(&entry).is_none();
let evicted = if new && self.lru.limiter().max_length() as usize <= self.lru.len() {
self.lru.pop_oldest().map(|(k, ())| k)
} else {
None
};
self.lru.get_or_insert(entry, || ());
evicted
}
/// Removes the given block from the buffer and also all the children of the block. /// Removes the given block from the buffer and also all the children of the block.
/// ///
/// This is used to get all the blocks that are dependent on the block that is included. /// This is used to get all the blocks that are dependent on the block that is included.
@ -164,7 +155,7 @@ impl<B: Block> BlockBuffer<B> {
let block = self.blocks.remove(hash)?; let block = self.blocks.remove(hash)?;
self.remove_from_earliest_blocks(block.number(), hash); self.remove_from_earliest_blocks(block.number(), hash);
self.remove_from_parent(block.parent_hash(), hash); self.remove_from_parent(block.parent_hash(), hash);
self.lru.remove(hash); self.block_queue.retain(|h| h != hash);
Some(block) Some(block)
} }
@ -213,7 +204,7 @@ mod tests {
/// Assert that all buffer collections have the same data length. /// Assert that all buffer collections have the same data length.
fn assert_buffer_lengths<B: Block>(buffer: &BlockBuffer<B>, expected: usize) { fn assert_buffer_lengths<B: Block>(buffer: &BlockBuffer<B>, expected: usize) {
assert_eq!(buffer.blocks.len(), expected); assert_eq!(buffer.blocks.len(), expected);
assert_eq!(buffer.lru.len(), expected); assert_eq!(buffer.block_queue.len(), expected);
assert_eq!( assert_eq!(
buffer.parent_to_child.iter().fold(0, |acc, (_, hashes)| acc + hashes.len()), buffer.parent_to_child.iter().fold(0, |acc, (_, hashes)| acc + hashes.len()),
expected expected