From e1a26d77c82382f4eecfa19d377313167146cbbd Mon Sep 17 00:00:00 2001
From: Marcus Holland-Moritz <github@mhxnet.de>
Date: Sun, 13 Aug 2023 07:18:59 +0200
Subject: [PATCH] Remove script ordering and more cleaning up

---
 include/dwarfs/inode_manager.h       |  8 +++-----
 include/dwarfs/options.h             |  2 +-
 src/dwarfs/fragment_order_parser.cpp |  3 ---
 src/dwarfs/inode_manager.cpp         | 20 +++----------------
 src/dwarfs/options.cpp               |  3 ---
 src/dwarfs/scanner.cpp               | 29 ++++++++++++++--------------
 test/dwarfs.cpp                      | 19 ++++++++----------
 7 files changed, 29 insertions(+), 55 deletions(-)

diff --git a/include/dwarfs/inode_manager.h b/include/dwarfs/inode_manager.h
index cdfa9f87..e183f1cf 100644
--- a/include/dwarfs/inode_manager.h
+++ b/include/dwarfs/inode_manager.h
@@ -55,9 +55,8 @@ class inode_manager {
 
   size_t count() const { return impl_->count(); }
 
-  void order_inodes(worker_group& wg, std::shared_ptr<script> scr,
-                    order_cb const& fn) {
-    impl_->order_inodes(wg, std::move(scr), fn);
+  void order_inodes(worker_group& wg, order_cb const& fn) {
+    impl_->order_inodes(wg, fn);
   }
 
   void for_each_inode_in_order(inode_cb const& fn) const {
@@ -88,8 +87,7 @@ class inode_manager {
 
     virtual std::shared_ptr<inode> create_inode() = 0;
     virtual size_t count() const = 0;
-    virtual void order_inodes(worker_group& wg, std::shared_ptr<script> scr,
-                              order_cb const& fn) = 0;
+    virtual void order_inodes(worker_group& wg, order_cb const& fn) = 0;
     virtual void for_each_inode_in_order(
         std::function<void(std::shared_ptr<inode> const&)> const& fn) const = 0;
     virtual std::vector<std::pair<fragment_category::value_type, size_t>>
diff --git a/include/dwarfs/options.h b/include/dwarfs/options.h
index 1ae1b0ae..4e5132c4 100644
--- a/include/dwarfs/options.h
+++ b/include/dwarfs/options.h
@@ -98,7 +98,7 @@ struct filesystem_writer_options {
 };
 
 // TODO: rename
-enum class file_order_mode { NONE, PATH, SCRIPT, SIMILARITY, NILSIMSA };
+enum class file_order_mode { NONE, PATH, SIMILARITY, NILSIMSA };
 
 // TODO: rename
 struct file_order_options {
diff --git a/src/dwarfs/fragment_order_parser.cpp b/src/dwarfs/fragment_order_parser.cpp
index 06fc6d11..fc490358 100644
--- a/src/dwarfs/fragment_order_parser.cpp
+++ b/src/dwarfs/fragment_order_parser.cpp
@@ -133,9 +133,6 @@ fragment_order_parser::to_string(file_order_options const& opts) const {
   case file_order_mode::PATH:
     return "path";
 
-  case file_order_mode::SCRIPT:
-    return "script";
-
   case file_order_mode::SIMILARITY:
     return "similarity";
 
diff --git a/src/dwarfs/inode_manager.cpp b/src/dwarfs/inode_manager.cpp
index d175bed0..6d5b21c9 100644
--- a/src/dwarfs/inode_manager.cpp
+++ b/src/dwarfs/inode_manager.cpp
@@ -340,7 +340,6 @@ class inode_ : public inode {
       switch (opts.fragment_order.get(f.category()).mode) {
       case file_order_mode::NONE:
       case file_order_mode::PATH:
-      case file_order_mode::SCRIPT:
         break;
       case file_order_mode::SIMILARITY:
         sc.try_emplace(f.category());
@@ -397,7 +396,6 @@ class inode_ : public inode {
     switch (order_mode) {
     case file_order_mode::NONE:
     case file_order_mode::PATH:
-    case file_order_mode::SCRIPT:
       break;
 
     case file_order_mode::SIMILARITY: {
@@ -469,8 +467,8 @@ class inode_manager_ final : public inode_manager::impl {
 
   size_t count() const override { return inodes_.size(); }
 
-  void order_inodes(worker_group& wg, std::shared_ptr<script> scr,
-                    inode_manager::order_cb const& fn) override;
+  void
+  order_inodes(worker_group& wg, inode_manager::order_cb const& fn) override;
 
   void for_each_inode_in_order(
       std::function<void(std::shared_ptr<inode> const&)> const& fn)
@@ -608,8 +606,7 @@ void inode_manager_<LoggerPolicy>::scan_background(worker_group& wg,
 
 template <typename LoggerPolicy>
 void inode_manager_<LoggerPolicy>::order_inodes(
-    worker_group& wg, std::shared_ptr<script> scr,
-    inode_manager::order_cb const& fn) {
+    worker_group& wg, inode_manager::order_cb const& fn) {
   // TODO: only use an index, never actually reorder inodes
 
   // TODO:
@@ -626,17 +623,6 @@ void inode_manager_<LoggerPolicy>::order_inodes(
     break;
   }
 
-  case file_order_mode::SCRIPT: {
-    if (!scr->has_order()) {
-      DWARFS_THROW(runtime_error, "script cannot order inodes");
-    }
-    LOG_INFO << "ordering " << count() << " inodes using script...";
-    auto ti = LOG_CPU_TIMED_INFO;
-    scr->order(inodes_);
-    ti << count() << " inodes ordered";
-    break;
-  }
-
   case file_order_mode::SIMILARITY: {
     LOG_INFO << "ordering " << count() << " inodes by similarity...";
     auto ti = LOG_CPU_TIMED_INFO;
diff --git a/src/dwarfs/options.cpp b/src/dwarfs/options.cpp
index fefd0848..19b98901 100644
--- a/src/dwarfs/options.cpp
+++ b/src/dwarfs/options.cpp
@@ -39,9 +39,6 @@ std::ostream& operator<<(std::ostream& os, file_order_mode mode) {
   case file_order_mode::PATH:
     modestr = "path";
     break;
-  case file_order_mode::SCRIPT:
-    modestr = "script";
-    break;
   case file_order_mode::SIMILARITY:
     modestr = "similarity";
     break;
diff --git a/src/dwarfs/scanner.cpp b/src/dwarfs/scanner.cpp
index 8b477aee..ab1172a4 100644
--- a/src/dwarfs/scanner.cpp
+++ b/src/dwarfs/scanner.cpp
@@ -670,21 +670,20 @@ void scanner_<LoggerPolicy>::scan(
       worker_group wg_order("ordering", num_threads);
 
       // ordering.add_job([&] {
-      im.order_inodes(wg_order, script_,
-                      [&](std::shared_ptr<inode> const& ino) {
-                        blockify.add_job([&, this] {
-                          prog.current.store(ino.get());
-                          inode_chunkable ic(*ino, *os_);
-                          seg.add_chunkable(ic);
-                          prog.inodes_written++;
-                        });
-                        auto queued_files = blockify.queue_size();
-                        auto queued_blocks = fsw.queue_fill();
-                        prog.blockify_queue = queued_files;
-                        prog.compress_queue = queued_blocks;
-                        return INT64_C(500) * queued_blocks +
-                               static_cast<int64_t>(queued_files);
-                      });
+      im.order_inodes(wg_order, [&](std::shared_ptr<inode> const& ino) {
+        blockify.add_job([&, this] {
+          prog.current.store(ino.get());
+          inode_chunkable ic(*ino, *os_);
+          seg.add_chunkable(ic);
+          prog.inodes_written++;
+        });
+        auto queued_files = blockify.queue_size();
+        auto queued_blocks = fsw.queue_fill();
+        prog.blockify_queue = queued_files;
+        prog.compress_queue = queued_blocks;
+        return INT64_C(500) * queued_blocks +
+               static_cast<int64_t>(queued_files);
+      });
       // });
 
       // wg_order.wait();
diff --git a/test/dwarfs.cpp b/test/dwarfs.cpp
index 7ec20c6f..f9d0f904 100644
--- a/test/dwarfs.cpp
+++ b/test/dwarfs.cpp
@@ -146,11 +146,7 @@ void basic_end_to_end_test(std::string const& compressor,
 
   auto prog = progress([](const progress&, bool) {}, 1000);
 
-  // TODO:
-  std::shared_ptr<script> scr;
-  if (file_order == file_order_mode::SCRIPT) {
-    scr = std::make_shared<test::script_mock>();
-  }
+  auto scr = std::make_shared<test::script_mock>();
 
   auto fsimage = build_dwarfs(lgr, input, compressor, cfg, options, &prog, scr);
   auto image_size = fsimage.size();
@@ -595,12 +591,13 @@ TEST_P(packing_test, regression_empty_fs) {
 
 INSTANTIATE_TEST_SUITE_P(
     dwarfs, compression_test,
-    ::testing::Combine(
-        ::testing::ValuesIn(compressions), ::testing::Values(12, 15, 20, 28),
-        ::testing::Values(file_order_mode::NONE, file_order_mode::PATH,
-                          file_order_mode::SCRIPT, file_order_mode::NILSIMSA,
-                          file_order_mode::SIMILARITY),
-        ::testing::Values(std::nullopt, "xxh3-128")));
+    ::testing::Combine(::testing::ValuesIn(compressions),
+                       ::testing::Values(12, 15, 20, 28),
+                       ::testing::Values(file_order_mode::NONE,
+                                         file_order_mode::PATH,
+                                         file_order_mode::NILSIMSA,
+                                         file_order_mode::SIMILARITY),
+                       ::testing::Values(std::nullopt, "xxh3-128")));
 
 INSTANTIATE_TEST_SUITE_P(
     dwarfs, scanner_test,