Merge pull request #172 from mingnus/2021-05-04-thin-dump-fixes

Make thin_dump runnable

src/pdata/btree_leaf_walker.rs

@@ -15,6 +15,7 @@ pub trait LeafVisitor<V: Unpack> {
     // Nodes may be shared and thus visited multiple times.  The walker avoids
     // doing repeated IO, but it does call this method to keep the visitor up to
     // date.  b may be an internal node obviously.
+    // FIXME: remove this method?
     fn visit_again(&mut self, b: u64) -> Result<()>;
     fn end_walk(&mut self) -> Result<()>;
 }
@@ -79,22 +80,13 @@ impl<'a> LeafWalker<'a> {
         V: Unpack,
     {
         assert_eq!(krs.len(), bs.len());
-        let mut errs: Vec<BTreeError> = Vec::new();
 
         let mut blocks = Vec::with_capacity(bs.len());
         let mut filtered_krs = Vec::with_capacity(krs.len());
         for i in 0..bs.len() {
-            if self.sm_inc(bs[i]) == 0 {
+            self.sm_inc(bs[i]);
-                // Node not yet seen
+            blocks.push(bs[i]);
-                blocks.push(bs[i]);
+            filtered_krs.push(krs[i].clone());
-                filtered_krs.push(krs[i].clone());
-            } else {
-                // This node has already been checked ...
-                if let Err(e) = visitor.visit_again(bs[i]) {
-                    // ... but the visitor isn't happy
-                    errs.push(e.clone());
-                }
-            }
         }
 
         let rblocks = self
@@ -145,6 +137,7 @@ impl<'a> LeafWalker<'a> {
         if let Internal { keys, values, .. } = node {
             let krs = split_key_ranges(path, &kr, &keys)?;
             if depth == 0 {
+                // it is the lowest internal
                 for i in 0..krs.len() {
                     self.sm.inc(values[i], 1).expect("sm.inc() failed");
                     for v in &values {
@@ -218,13 +211,13 @@ impl<'a> LeafWalker<'a> {
         let depth = self.get_depth::<V>(path, root, true)?;
 
         if depth == 0 {
+            // root is a leaf
             self.sm_inc(root);
             self.leaves.insert(root as usize);
             visitor.visit(&kr, root)?;
             Ok(())
-        } else if self.sm_inc(root) > 0 {
-            visitor.visit_again(root)
         } else {
+            self.sm_inc(root);
             let root = self.engine.read(root).map_err(|_| io_err(path))?;
 
             self.walk_node(depth - 1, path, visitor, &kr, &root, true)

src/thin/dump.rs

@@ -237,18 +237,16 @@ impl LeafVisitor<BlockTime> for CollectLeaves {
 
 fn collect_leaves(
     ctx: &Context,
-    shared: &mut BTreeSet<u64>,
+    roots: &BTreeSet<u64>,
     mut sm: Box<dyn SpaceMap>,
 ) -> Result<BTreeMap<u64, Vec<Entry>>> {
     let mut map: BTreeMap<u64, Vec<Entry>> = BTreeMap::new();
 
-    ctx.report.set_title(&format!(
+    ctx.report
-        "Collecting leaves for {} shared nodes",
+        .set_title(&format!("Collecting leaves for {} roots", roots.len()));
-        shared.len()
-    ));
 
     // FIXME: we don't want any leaves in shared.
-    for r in shared.iter() {
+    for r in roots.iter() {
         let old_count = sm.get(*r).expect("couldn't get count from space map.");
         sm.set(*r, 0).expect("couldn't set count in space map.");
 
@@ -270,6 +268,7 @@ fn collect_leaves(
 
 //------------------------------------------
 
+#[allow(dead_code)]
 fn find_shared_nodes(
     ctx: &Context,
     roots: &BTreeMap<u64, (Vec<u64>, u64)>,
@@ -335,16 +334,10 @@ fn build_metadata(ctx: &Context, sb: &Superblock) -> Result<Metadata> {
             btree_to_map_with_path::<u64>(&mut path, engine.clone(), sm, true, sb.mapping_root)?;
     }
 
-    report.set_title("Finding shared mappings");
+    let sm = Box::new(RestrictedSpaceMap::new(engine.get_nr_blocks()));
-    let (mut shared, sm) = find_shared_nodes(ctx, &roots)?;
+    let mapping_roots = roots.values().map(|(_, root)| *root).collect();
-
+    let entry_map = collect_leaves(&ctx, &mapping_roots, sm)?;
-    // Add in the roots, because they may not be shared.
+    let defs = Vec::new();
-    for (_path, root) in roots.values() {
-        shared.insert(*root);
-    }
-
-    let entry_map = collect_leaves(&ctx, &mut shared, sm)?;
-    let mut defs = Vec::new();
     let mut devs = Vec::new();
 
     let mut seen = BTreeSet::new();
@@ -364,20 +357,6 @@ fn build_metadata(ctx: &Context, sb: &Superblock) -> Result<Metadata> {
         });
     }
 
-    for b in shared {
-        if !seen.contains(&b) {
-            let es = entry_map.get(&b).unwrap();
-            let kr = KeyRange::new(); // FIXME: finish
-            defs.push(Def {
-                def_id: b,
-                map: Mapping {
-                    kr,
-                    entries: es.to_vec(),
-                },
-            });
-        }
-    }
-
     Ok(Metadata { defs, devs })
 }
 
@@ -485,9 +464,8 @@ fn optimise_metadata(md: Metadata) -> Result<Metadata> {
 
 //------------------------------------------
 
-fn emit_leaf(out: &mut dyn xml::MetadataVisitor, b: &Block) -> Result<()> {
+fn emit_leaf(v: &mut MappingVisitor, b: &Block) -> Result<()> {
     use Node::*;
-    let v = MappingVisitor::new(out);
     let path = Vec::new();
     let kr = KeyRange::new();
 
@@ -536,12 +514,14 @@ where
 }
 
 fn emit_leaves(ctx: &Context, out: &mut dyn xml::MetadataVisitor, ls: &[u64]) -> Result<()> {
+    let mut v = MappingVisitor::new(out);
     let proc = |b| {
-        emit_leaf(out, &b)?;
+        emit_leaf(&mut v, &b)?;
         Ok(())
     };
 
-    read_for(ctx.engine.clone(), ls, proc)
+    read_for(ctx.engine.clone(), ls, proc)?;
+    v.end_walk().map_err(|_| anyhow!("failed to emit leaves"))
 }
 
 fn emit_entries<W: Write>(
@@ -621,11 +601,10 @@ pub fn dump(opts: ThinDumpOptions) -> Result<()> {
     let sb = read_superblock(ctx.engine.as_ref(), SUPERBLOCK_LOCATION)?;
     let md = build_metadata(&ctx, &sb)?;
 
-    /*
     ctx.report
         .set_title("Optimising metadata to improve leaf packing");
     let md = optimise_metadata(md)?;
-    */
+
     dump_metadata(&ctx, &sb, &md)
 }
 

src/thin/runs.rs

@@ -19,6 +19,7 @@ pub struct Gatherer {
     heads: BTreeSet<u64>,
     tails: BTreeSet<u64>,
     entries: BTreeMap<u64, Entry>,
+    back: BTreeMap<u64, u64>,
 }
 
 impl Gatherer {
@@ -28,6 +29,7 @@ impl Gatherer {
             heads: BTreeSet::new(),
             tails: BTreeSet::new(),
             entries: BTreeMap::new(),
+            back: BTreeMap::new(),
         }
     }
 
@@ -59,6 +61,14 @@ impl Gatherer {
         if let Some(prev) = self.prev {
             let e = self.entries.get_mut(&prev).unwrap();
             e.neighbours.insert(b);
+
+            if let Some(back) = self.back.get(&b) {
+                if *back != prev {
+                    self.mark_head(b);
+                }
+            } else {
+                self.back.insert(b, prev);
+            }
         } else {
             self.mark_head(b);
         }
@@ -153,6 +163,8 @@ mod tests {
 
     #[test]
     fn gather() {
+        // the first value defines the input runs,
+        // and the second defines expected sub sequences
         struct Test(Vec<Vec<u64>>, Vec<Vec<u64>>);
 
         let tests = vec![
@@ -177,6 +189,10 @@ mod tests {
                 ],
                 vec![vec![1], vec![2], vec![3, 4], vec![5, 6]],
             ),
+            Test(
+                vec![vec![2, 3, 4, 5], vec![2, 3, 4, 6], vec![1, 3, 4, 5]],
+                vec![vec![1], vec![2], vec![3, 4], vec![5], vec![6]],
+            ),
         ];
 
         for t in tests {