[array_walker] Handle the whole array block at once

That gives the visitor more controls over the data processing and locking,
and also improves the performance by 10-15%.

src/cache/check.rs

@@ -8,7 +8,7 @@ use crate::cache::hint::*;
 use crate::cache::mapping::*;
 use crate::cache::superblock::*;
 use crate::io_engine::{AsyncIoEngine, IoEngine, SyncIoEngine};
-use crate::pdata::array;
+use crate::pdata::array::{self, ArrayBlock, ArrayError};
 use crate::pdata::array_walker::*;
 use crate::pdata::bitset_walker::*;
 
@@ -54,23 +54,40 @@ mod format1 {
             if m.oblock >= self.nr_origin_blocks {
                 return Err(array::value_err(format!("mapping beyond end of the origin device")));
             }
-
             let mut seen_oblocks = self.seen_oblocks.lock().unwrap();
             if seen_oblocks.contains(&m.oblock) {
                 return Err(array::value_err(format!("origin block already mapped")));
             }
             seen_oblocks.insert(m.oblock);
-
             Ok(())
         }
     }
 
     impl ArrayVisitor<Mapping> for MappingChecker {
-        fn visit(&self, _index: u64, m: Mapping) -> array::Result<()> {
-            self.check_flags(&m)?;
-            self.check_oblock(&m)?;
+        fn visit(&self, _index: u64, b: ArrayBlock<Mapping>) -> array::Result<()> {
+            let mut errs: Vec<ArrayError> = Vec::new();
 
-            Ok(())
+            for i in 0..b.header.nr_entries as usize {
+                let m = b.values[i];
+
+                if let Err(e) = self.check_flags(&m) {
+                    errs.push(e);
+                }
+                if let Err(e) = self.check_oblock(&m) {
+                    errs.push(e);
+                }
+            }
+
+            // FIXME: duplicate to BTreeWalker::build_aggregrate()
+            match errs.len() {
+                0 => Ok(()),
+                1 => {
+                    Err(errs[0].clone())
+                }
+                _ => {
+                    Err(array::aggregate_error(errs))
+                }
+            }
         }
     }
 }
@@ -129,12 +146,32 @@ mod format2 {
     }
 
     impl ArrayVisitor<Mapping> for MappingChecker {
-        fn visit(&self, index: u64, m: Mapping) -> array::Result<()> {
+        fn visit(&self, index: u64, b: ArrayBlock<Mapping>) -> array::Result<()> {
             let mut inner = self.inner.lock().unwrap();
-            self.check_flags(&m, inner.dirty_bits.contains(index as usize))?;
-            self.check_oblock(&m, &mut inner.seen_oblocks)?;
+            let mut errs: Vec<ArrayError> = Vec::new();
 
-            Ok(())
+            let begin = index as usize * b.header.max_entries as usize;
+            for i in 0..b.header.nr_entries {
+                let m = b.values[i as usize];
+
+                if let Err(e) = self.check_flags(&m, inner.dirty_bits.contains(begin + i as usize)) {
+                    errs.push(e);
+                }
+                if let Err(e) = self.check_oblock(&m, &mut inner.seen_oblocks) {
+                    errs.push(e);
+                }
+            }
+
+            // FIXME: duplicate to BTreeWalker::build_aggregrate()
+            match errs.len() {
+                0 => Ok(()),
+                1 => {
+                    Err(errs[0].clone())
+                }
+                _ => {
+                    Err(array::aggregate_error(errs))
+                }
+            }
         }
     }
 }
@@ -150,7 +187,7 @@ impl HintChecker {
 }
 
 impl ArrayVisitor<Hint> for HintChecker {
-    fn visit(&self, _index: u64, _hint: Hint) -> array::Result<()> {
+    fn visit(&self, _index: u64, _b: ArrayBlock<Hint>) -> array::Result<()> {
         // TODO: check hints
         Ok(())
     }

src/cache/dump.rs

@@ -8,7 +8,7 @@ use crate::cache::mapping::Mapping;
 use crate::cache::superblock::*;
 use crate::cache::xml::{self, MetadataVisitor};
 use crate::io_engine::{AsyncIoEngine, IoEngine, SyncIoEngine};
-use crate::pdata::array;
+use crate::pdata::array::{self, ArrayBlock};
 use crate::pdata::array_walker::*;
 
 //------------------------------------------
@@ -46,12 +46,17 @@ mod format1 {
     }
 
     impl<'a> ArrayVisitor<Mapping> for MappingEmitter<'a> {
-        fn visit(&self, index: u64, m: Mapping) -> array::Result<()> {
-            if m.is_valid() {
+        fn visit(&self, index: u64, b: ArrayBlock<Mapping>) -> array::Result<()> {
+            for i in 0..b.header.nr_entries as usize {
+                let map = b.values[i];
+                if !map.is_valid() {
+                    continue;
+                }
+
                 let m = xml::Map {
                     cblock: index as u32,
-                    oblock: m.oblock,
-                    dirty: m.is_dirty(),
+                    oblock: map.oblock,
+                    dirty: map.is_dirty(),
                 };
 
                 let mut inner = self.inner.lock().unwrap();
@@ -79,7 +84,7 @@ mod format2 {
     impl DirtyVisitor {
         pub fn new(nr_entries: usize) -> Self {
             DirtyVisitor {
-                nr_entries,
+                nr_entries, // number of bits
                 bits: Mutex::new(FixedBitSet::with_capacity(nr_entries)),
             }
         }
@@ -90,13 +95,19 @@ mod format2 {
     }
 
     impl ArrayVisitor<u64> for DirtyVisitor {
-        fn visit(&self, index: u64, bits: u64) -> array::Result<()> {
-            for i in 0..64u64 {
-                if (index + i) >= self.nr_entries as u64 {
-                    break;
-                }
+        fn visit(&self, index: u64, b: ArrayBlock<u64>) -> array::Result<()> {
+            let mut pos = index as usize * (b.header.max_entries as usize) << 6;
+            for i in 0..b.header.nr_entries as usize {
+                let bits = b.values[i];
 
-                self.bits.lock().unwrap().set((index + i) as usize, bits & (1 << i) != 0);
+                for bi in 0..64u64 {
+                    if pos >= self.nr_entries {
+                        break;
+                    }
+
+                    self.bits.lock().unwrap().set(pos, bits & (1 << bi) != 0);
+                    pos += 1;
+                }
             }
             Ok(())
         }
@@ -133,20 +144,25 @@ mod format2 {
     }
 
     impl<'a> ArrayVisitor<Mapping> for MappingEmitter<'a> {
-        fn visit(&self, index: u64, m: Mapping) -> array::Result<()> {
-            if m.is_valid() {
+        fn visit(&self, index: u64, b: ArrayBlock<Mapping>) -> array::Result<()> {
+            for i in 0..b.header.nr_entries as usize {
+                let map = b.values[i];
+
+                if !map.is_valid() {
+                    continue;
+                }
+
                 let mut inner = self.inner.lock().unwrap();
                 let dirty = inner.dirty_bits.contains(index as usize);
                 let m = xml::Map {
                     cblock: index as u32,
-                    oblock: m.oblock,
+                    oblock: map.oblock,
                     dirty,
                 };
 
                 inner.valid_mappings.set(index as usize, true);
                 inner.visitor.mapping(&m).map_err(|e| array::value_err(format!("{}", e)))?;
             }
-
             Ok(())
         }
     }
@@ -169,10 +185,16 @@ impl<'a> HintEmitter<'a> {
 }
 
 impl<'a> ArrayVisitor<Hint> for HintEmitter<'a> {
-    fn visit(&self, index: u64, hint: Hint) -> array::Result<()> {
-        if self.valid_mappings.contains(index as usize) {
+    fn visit(&self, index: u64, b: ArrayBlock<Hint>) -> array::Result<()> {
+        let mut cblock = index as u32 * b.header.max_entries;
+        for i in 0..b.header.nr_entries as usize {
+            if !self.valid_mappings.contains(cblock as usize) {
+                continue;
+            }
+
+            let hint = b.values[i];
             let h = xml::Hint {
-                cblock: index as u32,
+                cblock,
                 data: hint.hint.to_vec(),
             };
 
@@ -181,6 +203,8 @@ impl<'a> ArrayVisitor<Hint> for HintEmitter<'a> {
                 .unwrap()
                 .hint(&h)
                 .map_err(|e| array::value_err(format!("{}", e)))?;
+
+            cblock += 1;
         }
 
         Ok(())

src/pdata/array_walker.rs

@@ -14,7 +14,7 @@ pub struct ArrayWalker {
 }
 
 pub trait ArrayVisitor<V: Unpack> {
-    fn visit(&self, index: u64, v: V) -> array::Result<()>;
+    fn visit(&self, index: u64, b: ArrayBlock<V>) -> array::Result<()>;
 }
 
 //------------------------------------------
@@ -37,28 +37,6 @@ impl<'a, V: Unpack + Copy> BlockValueVisitor<'a, V> {
             array_errs: Mutex::new(Vec::new()),
         }
     }
-
-    fn visit_array_block(&self, index: u64, array_block: ArrayBlock<V>) -> array::Result<()>{
-        let mut errs: Vec<ArrayError> = Vec::new();
-
-        let begin = index * array_block.header.max_entries as u64;
-        for i in 0..array_block.header.nr_entries {
-            if let Err(e) = self.array_visitor.visit(begin + i as u64, array_block.values[i as usize]) {
-                errs.push(e); // TODO: add path or keys context?
-            }
-        }
-
-        // FIXME: duplicate to BTreeWalker::build_aggregrate()
-        match errs.len() {
-            0 => Ok(()),
-            1 => {
-                Err(errs[0].clone())
-            }
-            _ => {
-                Err(array::aggregate_error(errs))
-            }
-        }
-    }
 }
 
 impl<'a, V: Unpack + Copy> NodeVisitor<u64> for BlockValueVisitor<'a, V> {
@@ -89,7 +67,7 @@ impl<'a, V: Unpack + Copy> NodeVisitor<u64> for BlockValueVisitor<'a, V> {
             path.push(values[n]);
             match unpack_array_block::<V>(&path, b.get_data()) {
                 Ok(array_block) => {
-                    if let Err(e) = self.visit_array_block(*index, array_block) {
+                    if let Err(e) = self.array_visitor.visit(*index, array_block) {
                         self.array_errs.lock().unwrap().push(e);
                     }
                 },

src/pdata/bitset_walker.rs

@@ -2,7 +2,7 @@ use fixedbitset::FixedBitSet;
 use std::sync::{Arc, Mutex};
 
 use crate::io_engine::IoEngine;
-use crate::pdata::array;
+use crate::pdata::array::{self, ArrayBlock};
 use crate::pdata::array_walker::{ArrayVisitor, ArrayWalker};
 
 struct BitsetVisitor<'a> {
@@ -20,23 +20,28 @@ impl<'a> BitsetVisitor<'a> {
 }
 
 impl<'a> ArrayVisitor<u64> for BitsetVisitor<'a> {
-    fn visit(&self, index: u64, bits: u64) -> array::Result<()> {
-        let begin: u64 = index << 6;
-        if begin > self.nr_entries {
-            return Err(array::value_err("bitset size exceeds expectation".to_string()));
-        }
+    fn visit(&self, index: u64, b: ArrayBlock<u64>) -> array::Result<()> {
+        let mut begin = index as usize * (b.header.max_entries as usize) << 6;
 
-        let end: u64 = std::cmp::min(begin + 64, self.nr_entries);
-        let mut mask = 1;
-        for i in begin..end {
-            self.bits.lock().unwrap().set(i as usize, bits & mask != 0);
-            mask <<= 1;
+        for i in 0..b.header.nr_entries as usize {
+            if begin > self.nr_entries as usize {
+                return Err(array::value_err("bitset size exceeds expectation".to_string()));
+            }
+
+            let end: usize = std::cmp::min(begin + 64, self.nr_entries as usize);
+            let mut mask = 1;
+            let bits = b.values[i];
+
+            for bi in begin..end {
+                self.bits.lock().unwrap().set(bi, bits & mask != 0);
+                mask <<= 1;
+            }
+            begin += 64;
         }
         Ok(())
     }
 }
 
-// TODO: remap errors
 // TODO: multi-threaded is possible
 pub fn read_bitset(
     engine: Arc<dyn IoEngine + Send + Sync>,