Skip to content

Commit 22eda91

Browse files
authored
fix: don't clone blocks during get_successors (dfinity#3687)
Before: blocks were cloned prior to being serialized and returned to the `get_successors` caller. After: an `Arc` to the block is stored instead, and cloning doesn't happen (serialization does not need ownership). Block responses can get pretty large (~2MB), and cloning them take a long time. This is part of an effort to make the adapter able to process Testnet4 requests within the 50ms timeout from consensus.
1 parent f9a5f41 commit 22eda91

File tree

2 files changed

+10
-10
lines changed

2 files changed

+10
-10
lines changed

rs/bitcoin/adapter/src/blockchainstate.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use bitcoin::{
77

88
use ic_btc_validation::{validate_header, HeaderStore, ValidateHeaderError};
99
use ic_metrics::MetricsRegistry;
10-
use std::collections::HashMap;
10+
use std::{collections::HashMap, sync::Arc};
1111
use thiserror::Error;
1212

1313
use bitcoin::Work;
@@ -94,7 +94,7 @@ pub struct BlockchainState {
9494
header_cache: HashMap<BlockHash, HeaderNode>,
9595

9696
/// This field stores a hashmap containing BlockHash and the corresponding Block.
97-
block_cache: HashMap<BlockHash, Block>,
97+
block_cache: HashMap<BlockHash, Arc<Block>>,
9898

9999
/// This field contains the known tips of the header cache.
100100
tips: Vec<Tip>,
@@ -240,7 +240,7 @@ impl BlockchainState {
240240
.add_header(block.header)
241241
.map_err(AddBlockError::Header)?;
242242
self.tips.sort_unstable_by(|a, b| b.work.cmp(&a.work));
243-
self.block_cache.insert(block_hash, block);
243+
self.block_cache.insert(block_hash, Arc::new(block));
244244
self.metrics
245245
.block_cache_size
246246
.set(self.get_block_cache_size() as i64);
@@ -317,10 +317,10 @@ impl BlockchainState {
317317
hashes
318318
}
319319

320-
/// This method takes a list of block hashes as input.
321-
/// For each block hash, if the corresponding block is stored in the `block_cache`, the cached block is returned.
322-
pub fn get_block(&self, block_hash: &BlockHash) -> Option<&Block> {
323-
self.block_cache.get(block_hash)
320+
/// This method takes a block hash
321+
/// If the corresponding block is stored in the `block_cache`, the cached block is returned.
322+
pub fn get_block(&self, block_hash: &BlockHash) -> Option<Arc<Block>> {
323+
self.block_cache.get(block_hash).cloned()
324324
}
325325

326326
/// Used when the adapter is shutdown and no longer requires holding on to blocks.

rs/bitcoin/adapter/src/get_successors_handler.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ pub struct GetSuccessorsRequest {
5252
#[derive(Clone, Eq, PartialEq, Debug)]
5353
pub struct GetSuccessorsResponse {
5454
/// Blocks found in the block cache.
55-
pub blocks: Vec<Block>,
55+
pub blocks: Vec<Arc<Block>>,
5656
/// Next set of headers to be sent to the canister.
5757
pub next: Vec<BlockHeader>,
5858
}
@@ -151,7 +151,7 @@ fn get_successor_blocks(
151151
anchor: &BlockHash,
152152
processed_block_hashes: &[BlockHash],
153153
allow_multiple_blocks: bool,
154-
) -> Vec<Block> {
154+
) -> Vec<Arc<Block>> {
155155
let seen: HashSet<BlockHash> = processed_block_hashes.iter().copied().collect();
156156

157157
let mut successor_blocks = vec![];
@@ -204,7 +204,7 @@ fn get_next_headers(
204204
state: &BlockchainState,
205205
anchor: &BlockHash,
206206
processed_block_hashes: &[BlockHash],
207-
blocks: &[Block],
207+
blocks: &[Arc<Block>],
208208
) -> Vec<BlockHeader> {
209209
let seen: HashSet<BlockHash> = processed_block_hashes
210210
.iter()

0 commit comments

Comments
 (0)