Merge pull request #3953 from quake/quake/optimize-get-ancestor

refactor: use BlockNumberAndHash in get_ancestor
nervosnetwork · Apr 23, 2023 · 0c4565b · 0c4565b
2 parents 5f3a6bb + 087e742
commit 0c4565b
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 19 deletions.
diff --git a/sync/src/tests/types.rs b/sync/src/tests/types.rs
@@ -5,7 +5,10 @@ use ckb_types::{
     U256,
 };
 use rand::{thread_rng, Rng};
-use std::collections::{BTreeMap, HashMap};
+use std::{
+    collections::{BTreeMap, HashMap},
+    sync::atomic::{AtomicUsize, Ordering::Relaxed},
+};
 
 use crate::types::{HeaderView, TtlFilter, FILTER_TTL};
 
@@ -51,7 +54,7 @@ fn test_get_ancestor_use_skip_list() {
 
     let mut rng = thread_rng();
     let a_to_b = |a, b, limit| {
-        let mut count = 0;
+        let count = AtomicUsize::new(0);
         let header = header_map
             .get(&hashes[&a])
             .cloned()
@@ -60,15 +63,15 @@ fn test_get_ancestor_use_skip_list() {
                 0,
                 b,
                 |hash, _| {
-                    count += 1;
+                    count.fetch_add(1, Relaxed);
                     header_map.get(hash).cloned()
                 },
                 |_, _| None,
             )
             .unwrap();
 
         // Search must finished in <limit> steps
-        assert!(count <= limit);
+        assert!(count.load(Relaxed) <= limit);
 
         header
     };

diff --git a/sync/src/types/mod.rs b/sync/src/types/mod.rs
@@ -1046,14 +1046,13 @@ impl HeaderView {
 
     pub fn build_skip<F, G>(&mut self, tip_number: BlockNumber, get_header_view: F, fast_scanner: G)
     where
-        F: FnMut(&Byte32, Option<bool>) -> Option<HeaderView>,
-        G: Fn(BlockNumber, &HeaderView) -> Option<HeaderView>,
+        F: Fn(&Byte32, Option<bool>) -> Option<HeaderView>,
+        G: Fn(BlockNumber, BlockNumberAndHash) -> Option<HeaderView>,
     {
         if self.inner.is_genesis() {
             return;
         }
         self.skip_hash = self
-            .clone()
             .get_ancestor(
                 tip_number,
                 get_skip_height(self.number()),
@@ -1063,23 +1062,22 @@ impl HeaderView {
             .map(|header| header.hash());
     }
 
-    // NOTE: get_header_view may change source state, for cache or for tests
     pub fn get_ancestor<F, G>(
-        self,
+        &self,
         tip_number: BlockNumber,
         number: BlockNumber,
-        mut get_header_view: F,
+        get_header_view: F,
         fast_scanner: G,
     ) -> Option<core::HeaderView>
     where
-        F: FnMut(&Byte32, Option<bool>) -> Option<HeaderView>,
-        G: Fn(BlockNumber, &HeaderView) -> Option<HeaderView>,
+        F: Fn(&Byte32, Option<bool>) -> Option<HeaderView>,
+        G: Fn(BlockNumber, BlockNumberAndHash) -> Option<HeaderView>,
     {
-        let mut current = self;
-        if number > current.number() {
+        if number > self.number() {
             return None;
         }
 
+        let mut current = self.clone();
         let mut number_walk = current.number();
         while number_walk > number {
             let number_skip = get_skip_height(number_walk);
@@ -1101,7 +1099,7 @@ impl HeaderView {
                     number_walk -= 1;
                 }
             }
-            if let Some(target) = fast_scanner(number, &current) {
+            if let Some(target) = fast_scanner(number, (current.number(), current.hash()).into()) {
                 current = target;
                 break;
             }
@@ -1440,8 +1438,7 @@ impl SyncShared {
             |hash, store_first_opt| self.get_header_view(hash, store_first_opt),
             |number, current| {
                 // shortcut to return an ancestor block
-                if current.number() <= snapshot.tip_number()
-                    && snapshot.is_main_chain(&current.hash())
+                if current.number <= snapshot.tip_number() && snapshot.is_main_chain(&current.hash)
                 {
                     snapshot
                         .get_block_hash(number)
@@ -2019,8 +2016,7 @@ impl ActiveChain {
             |hash, store_first_opt| self.shared.get_header_view(hash, store_first_opt),
             |number, current| {
                 // shortcut to return an ancestor block
-                if current.number() <= tip_number && self.snapshot().is_main_chain(&current.hash())
-                {
+                if current.number <= tip_number && self.snapshot().is_main_chain(&current.hash) {
                     self.get_block_hash(number)
                         .and_then(|hash| self.shared.get_header_view(&hash, Some(true)))
                 } else {