From 5858c1c53deea79db2ff14983dbcba1de4416252 Mon Sep 17 00:00:00 2001
From: Marcus Holland-Moritz <github@mhxnet.de>
Date: Sat, 28 Nov 2020 01:40:29 +0100
Subject: [PATCH] metadata_v2: clean up inode_manager

---
 include/dwarfs/fstypes.h       | 65 ----------------------------------
 include/dwarfs/inode.h         |  3 +-
 include/dwarfs/inode_manager.h | 11 ++----
 src/dwarfs/inode_manager.cpp   | 52 +++++----------------------
 src/dwarfs/scanner.cpp         | 16 ++++-----
 5 files changed, 20 insertions(+), 127 deletions(-)

diff --git a/include/dwarfs/fstypes.h b/include/dwarfs/fstypes.h
index 80dbc3f8..4554f3fe 100644
--- a/include/dwarfs/fstypes.h
+++ b/include/dwarfs/fstypes.h
@@ -132,71 +132,6 @@ struct directory {
   } u;
 };
 
-struct meta_config {
-  uint8_t block_size_bits;
-  dir_entry_type de_type;
-  uint16_t unused;
-  uint32_t inode_count;
-  uint64_t orig_fs_size;
-  uint32_t chunk_index_offset;
-  uint32_t inode_index_offset;
-};
-
-using chunk_type = uint64_t;
-
-template <unsigned BlockSizeBits>
-struct chunk_access {
-  static_assert(BlockSizeBits < 32, "invalid value for BlockSizeBits");
-
-  static const unsigned block_bits = 64 - 2 * BlockSizeBits;
-  static const unsigned block_shift = 64 - block_bits;
-  static const chunk_type block_mask =
-      (static_cast<chunk_type>(1) << block_bits) - 1;
-  static const unsigned offset_shift = BlockSizeBits;
-  static const chunk_type offset_mask =
-      (static_cast<chunk_type>(1) << BlockSizeBits) - 1;
-  static const unsigned size_shift = 0;
-  static const chunk_type size_mask =
-      (static_cast<chunk_type>(1) << BlockSizeBits) - 1;
-  static const chunk_type max_size = size_mask + 1;
-
-  static void set(chunk_type& chunk, size_t block, size_t offset, size_t size) {
-    if (block > block_mask) {
-      std::cerr << "block out of range: " << block << " > " << block_mask
-                << " [" << block_bits << "]";
-      throw std::runtime_error("block out of range");
-    }
-
-    if (offset > offset_mask) {
-      std::cerr << "offset out of range: " << offset << " > " << offset_mask
-                << " [" << block_bits << "]";
-      throw std::runtime_error("offset out of range");
-    }
-
-    if (size > max_size or size == 0) {
-      std::cerr << "size out of range: " << size << " > " << size_mask << " ["
-                << block_bits << "]";
-      throw std::runtime_error("size out of range");
-    }
-
-    chunk = (static_cast<chunk_type>(block) << block_shift) |
-            (static_cast<chunk_type>(offset) << offset_shift) |
-            (static_cast<chunk_type>(size - 1) << size_shift);
-  }
-
-  static size_t block(chunk_type chunk) {
-    return (chunk >> block_shift) & block_mask;
-  };
-
-  static size_t offset(chunk_type chunk) {
-    return (chunk >> offset_shift) & offset_mask;
-  };
-
-  static size_t size(chunk_type chunk) {
-    return ((chunk >> size_shift) & size_mask) + 1;
-  };
-};
-
 std::string get_compression_name(compression_type type);
 
 std::string get_section_name(section_type type);
diff --git a/include/dwarfs/inode.h b/include/dwarfs/inode.h
index 06899890..14ab3e22 100644
--- a/include/dwarfs/inode.h
+++ b/include/dwarfs/inode.h
@@ -41,8 +41,7 @@ class inode : public file_interface {
   virtual uint32_t similarity_hash() const = 0;
   virtual const file_interface* any() const = 0; // TODO
   virtual void add_chunk(size_t block, size_t offset, size_t size) = 0;
-  virtual const std::vector<chunk_type>& chunks() const = 0;
   virtual void
-  append_chunks(std::vector<thrift::metadata::chunk>& vec) const = 0;
+  append_chunks_to(std::vector<thrift::metadata::chunk>& vec) const = 0;
 };
 } // namespace dwarfs
diff --git a/include/dwarfs/inode_manager.h b/include/dwarfs/inode_manager.h
index f882c893..5971fb72 100644
--- a/include/dwarfs/inode_manager.h
+++ b/include/dwarfs/inode_manager.h
@@ -34,23 +34,16 @@ class script;
 
 class inode_manager {
  public:
-  static std::shared_ptr<inode_manager> create(unsigned block_size_bits);
+  static std::unique_ptr<inode_manager> create();
 
   virtual ~inode_manager() = default;
-  virtual std::shared_ptr<inode> create() = 0;
+  virtual std::shared_ptr<inode> create_inode() = 0;
   virtual size_t count() const = 0;
-  virtual size_t block_size() const = 0;
-  virtual unsigned block_size_bits() const = 0;
-  virtual size_t chunk_size() const = 0;
   virtual void order_inodes() = 0;
   virtual void order_inodes(std::shared_ptr<script> scr) = 0;
   virtual void order_inodes_by_similarity() = 0;
   virtual void number_inodes(size_t first_no) = 0;
   virtual void for_each_inode(
       std::function<void(std::shared_ptr<inode> const&)> const& fn) const = 0;
-
- private:
-  template <unsigned BlockSizeBits>
-  static std::shared_ptr<inode_manager> create_(unsigned block_size_bits);
 };
 } // namespace dwarfs
diff --git a/src/dwarfs/inode_manager.cpp b/src/dwarfs/inode_manager.cpp
index cf665aae..2563e39b 100644
--- a/src/dwarfs/inode_manager.cpp
+++ b/src/dwarfs/inode_manager.cpp
@@ -32,12 +32,11 @@
 
 namespace dwarfs {
 
-template <unsigned BlockSizeBits = 24>
 class inode_manager_ : public inode_manager {
  private:
   class inode_ : public inode {
    public:
-    using access = chunk_access<BlockSizeBits>;
+    using chunk_type = thrift::metadata::chunk;
 
     void set_num(uint32_t num) override { num_ = num; }
     uint32_t num() const override { return num_; }
@@ -59,7 +58,9 @@ class inode_manager_ : public inode_manager {
 
     void add_chunk(size_t block, size_t offset, size_t size) override {
       chunk_type c;
-      access::set(c, block, offset, size);
+      c.block = block;
+      c.offset = offset;
+      c.size = size;
       chunks_.push_back(c);
     }
 
@@ -76,17 +77,9 @@ class inode_manager_ : public inode_manager {
       return file_;
     }
 
-    const std::vector<chunk_type>& chunks() const override { return chunks_; }
-
     void
-    append_chunks(std::vector<thrift::metadata::chunk>& vec) const override {
-      for (auto c : chunks_) {
-        thrift::metadata::chunk chnk;
-        chnk.block = access::block(c);
-        chnk.offset = access::offset(c);
-        chnk.size = access::size(c);
-        vec.push_back(chnk);
-      }
+    append_chunks_to(std::vector<chunk_type>& vec) const override {
+      vec.insert(vec.end(), chunks_.begin(), chunks_.end());
     }
 
    private:
@@ -96,7 +89,7 @@ class inode_manager_ : public inode_manager {
   };
 
  public:
-  std::shared_ptr<inode> create() override {
+  std::shared_ptr<inode> create_inode() override {
     auto ino = std::make_shared<inode_>();
     inodes_.push_back(ino);
     return ino;
@@ -104,17 +97,6 @@ class inode_manager_ : public inode_manager {
 
   size_t count() const override { return inodes_.size(); }
 
-  size_t block_size() const override {
-    return static_cast<size_t>(1) << BlockSizeBits;
-  }
-
-  unsigned block_size_bits() const override { return BlockSizeBits; }
-
-  size_t chunk_size() const override {
-    // TODO: not needed
-    return sizeof(chunk_type);
-  }
-
   void order_inodes(std::shared_ptr<script> scr) override {
     scr->order(inodes_);
   }
@@ -175,23 +157,7 @@ class inode_manager_ : public inode_manager {
   std::vector<std::shared_ptr<inode>> inodes_;
 };
 
-template <unsigned BlockSizeBits>
-std::shared_ptr<inode_manager>
-inode_manager::create_(unsigned block_size_bits) {
-  if (block_size_bits == BlockSizeBits) {
-    return std::make_shared<inode_manager_<BlockSizeBits>>();
-  }
-
-  return create_<BlockSizeBits - 1>(block_size_bits);
-}
-
-template <>
-std::shared_ptr<inode_manager>
-inode_manager::create_<MIN_BLOCK_BITS_SIZE - 1>(unsigned) {
-  throw std::runtime_error("unsupported block_size_bits");
-}
-
-std::shared_ptr<inode_manager> inode_manager::create(unsigned block_size_bits) {
-  return create_<MAX_BLOCK_BITS_SIZE>(block_size_bits);
+std::unique_ptr<inode_manager> inode_manager::create() {
+  return std::make_unique<inode_manager_>();
 }
 } // namespace dwarfs
diff --git a/src/dwarfs/scanner.cpp b/src/dwarfs/scanner.cpp
index 9e335e71..0079c70b 100644
--- a/src/dwarfs/scanner.cpp
+++ b/src/dwarfs/scanner.cpp
@@ -299,7 +299,7 @@ void scanner_<LoggerPolicy>::scan(filesystem_writer& fsw,
   link_set_inode_visitor lsiv(first_file_inode);
   root->accept(lsiv, true);
 
-  auto im = inode_manager::create(cfg_.block_size_bits);
+  auto im = inode_manager::create();
 
   for (auto& p : file_hash) {
     if (p.second.size() > 1) {
@@ -310,7 +310,7 @@ void scanner_<LoggerPolicy>::scan(filesystem_writer& fsw,
 
     auto first = p.second.front();
     {
-      auto inode = im->create();
+      auto inode = im->create_inode();
       first->set_inode(inode);
       inode->set_file(first);
     }
@@ -395,13 +395,13 @@ void scanner_<LoggerPolicy>::scan(filesystem_writer& fsw,
     prog.inodes_written++;
   });
 
-  prog.sync([&] { prog.current.store(nullptr); });
-
-  log_.debug() << "waiting for block compression to finish...";
+  log_.info() << "waiting for block compression to finish...";
 
   bm.finish_blocks();
   wg_.wait();
 
+  prog.sync([&] { prog.current.store(nullptr); });
+
   // TODO: check this, doesn't seem to come out right in debug output
   //       seems to be out-of-line with block compression??
   log_.debug() << "compressed " << size_with_unit(bm.total_size()) << " in "
@@ -425,7 +425,7 @@ void scanner_<LoggerPolicy>::scan(filesystem_writer& fsw,
   //       submitted for compression
   im->for_each_inode([&](std::shared_ptr<inode> const& ino) {
     mv2.chunk_index.at(ino->num() - first_file_inode) = mv2.chunks.size();
-    ino->append_chunks(mv2.chunks);
+    ino->append_chunks_to(mv2.chunks);
   });
 
   // insert dummy inode to help determine number of chunks per inode
@@ -455,7 +455,7 @@ void scanner_<LoggerPolicy>::scan(filesystem_writer& fsw,
   mv2.links = ge_data.get_links();
   mv2.timestamp_base = ge_data.timestamp_base;
   mv2.chunk_index_offset = first_file_inode;
-  mv2.block_size = im->block_size();
+  mv2.block_size = UINT32_C(1) << cfg_.block_size_bits;
   mv2.total_fs_size = prog.original_size;
 
   auto [schema, data] = metadata_v2::freeze(mv2);
@@ -463,7 +463,7 @@ void scanner_<LoggerPolicy>::scan(filesystem_writer& fsw,
   fsw.write_metadata_v2_schema(std::move(schema));
   fsw.write_metadata_v2(std::move(data));
 
-  log_.info() << "waiting for compression tasks to finish...";
+  log_.info() << "waiting for compression to finish...";
 
   fsw.flush();