Skip to content

Commit

Permalink
Use a struct for runtime service pinned blocks (#2723)
Browse files Browse the repository at this point in the history 
Minor refactoring to help with clarity of the code.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
@tomaka @mergify
tomaka and mergify[bot] authored Sep 12, 2022
1 parent 2e756d4 commit dd7619b
Showing 1 changed file with 46 additions and 31 deletions.
77 changes: 46 additions & 31 deletions bin/light-base/src/runtime_service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,12 @@ impl<TPlat: Platform> RuntimeService<TPlat> {

let _prev_value = pinned_blocks.insert(
(subscription_id, finalized_block.hash),
(
tree.finalized_async_user_data().clone(),
*decoded_finalized_block.state_root,
decoded_finalized_block.number,
false,
),
PinnedBlock {
runtime: tree.finalized_async_user_data().clone(),
state_trie_root_hash: *decoded_finalized_block.state_root,
block_number: decoded_finalized_block.number,
block_ignores_limit: false,
},
);
debug_assert!(_prev_value.is_none());

Expand Down Expand Up @@ -314,12 +314,12 @@ impl<TPlat: Platform> RuntimeService<TPlat> {

let _prev_value = pinned_blocks.insert(
(subscription_id, block_hash),
(
runtime.clone(),
*decoded_header.state_root,
decoded_header.number,
true,
),
PinnedBlock {
runtime: runtime.clone(),
state_trie_root_hash: *decoded_header.state_root,
block_number: decoded_header.number,
block_ignores_limit: true,
},
);
debug_assert!(_prev_value.is_none());

Expand Down Expand Up @@ -400,10 +400,9 @@ impl<TPlat: Platform> RuntimeService<TPlat> {
..
} = &mut guarded_lock.tree
{
let block_counts_towards_limit = match pinned_blocks
.remove(&(subscription_id.0, *block_hash))
let block_ignores_limit = match pinned_blocks.remove(&(subscription_id.0, *block_hash))
{
Some((_, _, _, to_remove)) => !to_remove,
Some(b) => b.block_ignores_limit,
None => {
// Cold path.
if let Some((sub_name, _, _)) = all_blocks_subscriptions.get(&subscription_id.0)
Expand All @@ -417,7 +416,7 @@ impl<TPlat: Platform> RuntimeService<TPlat> {

guarded_lock.runtimes.retain(|_, rt| rt.strong_count() > 0);

if block_counts_towards_limit {
if !block_ignores_limit {
let (_name, _, finalized_pinned_remaining) = all_blocks_subscriptions
.get_mut(&subscription_id.0)
.unwrap();
Expand Down Expand Up @@ -451,7 +450,7 @@ impl<TPlat: Platform> RuntimeService<TPlat> {
let mut guarded = self.guarded.lock().await;
let guarded = &mut *guarded;

let (runtime, block_state_root_hash, block_number, _) = {
let pinned_block = {
if let GuardedInner::FinalizedBlockRuntimeKnown {
all_blocks_subscriptions,
pinned_blocks,
Expand Down Expand Up @@ -479,9 +478,9 @@ impl<TPlat: Platform> RuntimeService<TPlat> {
Ok(RuntimeLock {
service: self,
hash: block_hash,
runtime,
block_number,
block_state_root_hash,
runtime: pinned_block.runtime,
block_number: pinned_block.block_number,
block_state_root_hash: pinned_block.state_trie_root_hash,
})
}

Expand Down Expand Up @@ -1047,8 +1046,7 @@ enum GuardedInner<TPlat: Platform> {
/// Keys are `(subscription_id, block_hash)`. Values are indices within
/// [`Guarded::runtimes`], state trie root hashes, block numbers, and whether the block
/// is non-finalized and part of the canonical chain.
// TODO: use structs instead of tuples
pinned_blocks: BTreeMap<(u64, [u8; 32]), (Arc<Runtime>, [u8; 32], u64, bool)>,
pinned_blocks: BTreeMap<(u64, [u8; 32]), PinnedBlock>,
},
FinalizedBlockRuntimeUnknown {
/// Tree of blocks. Holds the state of the download of everything. Always `Some` when the
Expand All @@ -1069,6 +1067,23 @@ enum GuardedInner<TPlat: Platform> {
},
}

#[derive(Clone)]
struct PinnedBlock {
/// Reference-counted runtime of the pinned block.
runtime: Arc<Runtime>,

/// Hash of the trie root of the pinned block.
state_trie_root_hash: [u8; 32],

/// Height of the pinned block.
block_number: u64,

/// `true` if the block is non-finalized and part of the canonical chain.
/// If `true`, then the block doesn't count towards the maximum number of pinned blocks of
/// the subscription.
block_ignores_limit: bool,
}

#[derive(Clone)]
struct Block {
/// Hash of the block in question. Redundant with `header`, but the hash is so often needed
Expand Down Expand Up @@ -1623,11 +1638,11 @@ impl<TPlat: Platform> Background<TPlat> {
for block in iter::once(&finalized_block.hash)
.chain(pruned_blocks.iter().map(|(_, b, _)| &b.hash))
{
if let Some((_, _, _, non_finalized_canonical)) =
if let Some(pin) =
pinned_blocks.get_mut(&(*subscription_id, *block))
{
debug_assert!(*non_finalized_canonical);
*non_finalized_canonical = false;
debug_assert!(pin.block_ignores_limit);
pin.block_ignores_limit = false;
}
}
}
Expand All @@ -1649,7 +1664,7 @@ impl<TPlat: Platform> Background<TPlat> {
let scale_encoded_header = block.user_data.scale_encoded_header.clone();
let is_new_best = block.is_new_best;

let (block_number, block_state_root_hash) = {
let (block_number, state_trie_root_hash) = {
let decoded = header::decode(
&scale_encoded_header,
self.sync_service.block_number_bytes(),
Expand Down Expand Up @@ -1696,12 +1711,12 @@ impl<TPlat: Platform> Background<TPlat> {
if sender.try_send(notif.clone()).is_ok() {
let _prev_value = pinned_blocks.insert(
(*subscription_id, block_hash),
(
block_runtime.clone(),
block_state_root_hash,
PinnedBlock {
runtime: block_runtime.clone(),
state_trie_root_hash,
block_number,
true,
),
block_ignores_limit: true,
},
);
debug_assert!(_prev_value.is_none());
} else {
Expand Down

0 comments on commit dd7619b

Please sign in to comment.