crimson/os/seastore: cleanup effort_t and reuse Transaction::io_stat_t

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

3 files changed, 60 insertions, 64 deletions

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 4b7ab5495af..67273d7b0d6 100644
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -212,7 +212,7 @@ void Cache::register_metrics()
           {
             sm::make_counter(
               "invalidated_extents",
-              efforts.read.extents,
+              efforts.read.num,
               sm::description("extents of invalidated transactions"),
               {src_label, read_effort_label}
             ),
@@ -229,7 +229,7 @@ void Cache::register_metrics()
 
       // non READ invalidated efforts
       for (auto& effort_name : invalidated_effort_names) {
-        auto& effort = [&effort_name, &efforts]() -> effort_t& {
+        auto& effort = [&effort_name, &efforts]() -> io_stat_t& {
           if (effort_name == "READ") {
             return efforts.read;
           } else if (effort_name == "MUTATE") {
@@ -248,7 +248,7 @@ void Cache::register_metrics()
           {
             sm::make_counter(
               "invalidated_extents",
-              effort.extents,
+              effort.num,
               sm::description("extents of invalidated transactions"),
               {src_label, effort_label(effort_name)}
             ),
@@ -346,7 +346,7 @@ void Cache::register_metrics()
       );
       for (auto& effort_name : committed_effort_names) {
         auto& effort_by_ext = [&efforts, &effort_name]()
-            -> counter_by_extent_t<effort_t>& {
+            -> counter_by_extent_t<io_stat_t>& {
           if (effort_name == "READ") {
             return efforts.read_by_ext;
           } else if (effort_name == "MUTATE") {
@@ -369,7 +369,7 @@ void Cache::register_metrics()
             {
               sm::make_counter(
                 "committed_extents",
-                effort.extents,
+                effort.num,
                 sm::description("extents of committed transactions"),
                 {src_label, effort_label(effort_name), ext_label}
               ),
@@ -412,7 +412,7 @@ void Cache::register_metrics()
         ),
         sm::make_counter(
           "successful_read_extents",
-          stats.success_read_efforts.read.extents,
+          stats.success_read_efforts.read.num,
           sm::description("extents of successful read transactions")
         ),
         sm::make_counter(
@@ -762,33 +762,35 @@ void Cache::mark_transaction_conflicted(
                              conflicting_extent.get_type());
   ++counter;
 
-  efforts.read.extents += t.read_set.size();
+  io_stat_t read_stat;
   for (auto &i: t.read_set) {
-    efforts.read.bytes += i.ref->get_length();
+    read_stat.increment(i.ref->get_length());
   }
+  efforts.read.increment_stat(read_stat);
 
   if (t.get_src() != Transaction::src_t::READ) {
-    efforts.retire.extents += t.retired_set.size();
+    io_stat_t retire_stat;
     for (auto &i: t.retired_set) {
-      efforts.retire.bytes += i->get_length();
+      retire_stat.increment(i->get_length());
     }
+    efforts.retire.increment_stat(retire_stat);
 
-    auto& fresh_stats = t.get_fresh_block_stats();
-    efforts.fresh.extents += fresh_stats.num;
-    efforts.fresh.bytes += fresh_stats.bytes;
+    auto& fresh_stat = t.get_fresh_block_stats();
+    efforts.fresh.increment_stat(fresh_stat);
 
+    io_stat_t delta_stat;
     for (auto &i: t.mutated_block_list) {
       if (!i->is_valid()) {
         continue;
       }
       DEBUGT("was mutating extent: {}", t, *i);
       efforts.mutate.increment(i->get_length());
-      efforts.mutate_delta_bytes += i->get_delta().length();
+      delta_stat.increment(i->get_delta().length());
     }
+    efforts.mutate_delta_bytes += delta_stat.bytes;
 
     auto& ool_stats = t.get_ool_write_stats();
-    efforts.fresh_ool_written.extents += ool_stats.extents.num;
-    efforts.fresh_ool_written.bytes += ool_stats.extents.bytes;
+    efforts.fresh_ool_written.increment_stat(ool_stats.extents);
     efforts.num_ool_records += ool_stats.num_records;
     efforts.ool_record_bytes +=
       (ool_stats.header_bytes + ool_stats.data_bytes);
@@ -823,16 +825,17 @@ void Cache::on_transaction_destruct(Transaction& t)
       t.conflicted == false &&
       !t.is_weak()) {
     DEBUGT("read is successful", t);
-    ++stats.success_read_efforts.num_trans;
-
-    auto& effort = stats.success_read_efforts.read;
-    effort.extents += t.read_set.size();
+    io_stat_t read_stat;
     for (auto &i: t.read_set) {
-      effort.bytes += i.ref->get_length();
+      read_stat.increment(i.ref->get_length());
     }
+
+    ++stats.success_read_efforts.num_trans;
+    stats.success_read_efforts.read.increment_stat(read_stat);
+
     // read transaction won't have non-read efforts
     assert(t.retired_set.empty());
-    assert(t.get_fresh_block_stats().num == 0);
+    assert(t.get_fresh_block_stats().is_clear());
     assert(t.mutated_block_list.empty());
     assert(t.onode_tree_stats.is_clear());
     assert(t.lba_tree_stats.is_clear());
diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 2c50c0e9316..a7de00d807d 100644
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -795,49 +795,29 @@ private:
     uint64_t hit = 0;
   };
 
-  /**
-   * effort_t
-   *
-   * Count the number of extents involved in the effort and the total bytes of
-   * them.
-   *
-   * Each effort_t represents the effort of a set of extents involved in the
-   * transaction, classified by read, mutate, retire and allocate behaviors,
-   * see XXX_trans_efforts_t.
-   */
-  struct effort_t {
-    uint64_t extents = 0;
-    uint64_t bytes = 0;
-
-    void increment(uint64_t extent_len) {
-      ++extents;
-      bytes += extent_len;
-    }
-  };
-
   template <typename CounterT>
   using counter_by_extent_t = std::array<CounterT, EXTENT_TYPES_MAX>;
 
   struct invalid_trans_efforts_t {
-    effort_t read;
-    effort_t mutate;
+    io_stat_t read;
+    io_stat_t mutate;
     uint64_t mutate_delta_bytes = 0;
-    effort_t retire;
-    effort_t fresh;
-    effort_t fresh_ool_written;
+    io_stat_t retire;
+    io_stat_t fresh;
+    io_stat_t fresh_ool_written;
     counter_by_extent_t<uint64_t> num_trans_invalidated;
     uint64_t num_ool_records = 0;
     uint64_t ool_record_bytes = 0;
   };
 
   struct commit_trans_efforts_t {
-    counter_by_extent_t<effort_t> read_by_ext;
-    counter_by_extent_t<effort_t> mutate_by_ext;
+    counter_by_extent_t<io_stat_t> read_by_ext;
+    counter_by_extent_t<io_stat_t> mutate_by_ext;
     counter_by_extent_t<uint64_t> delta_bytes_by_ext;
-    counter_by_extent_t<effort_t> retire_by_ext;
-    counter_by_extent_t<effort_t> fresh_invalid_by_ext; // inline but is already invalid (retired)
-    counter_by_extent_t<effort_t> fresh_inline_by_ext;
-    counter_by_extent_t<effort_t> fresh_ool_by_ext;
+    counter_by_extent_t<io_stat_t> retire_by_ext;
+    counter_by_extent_t<io_stat_t> fresh_invalid_by_ext; // inline but is already invalid (retired)
+    counter_by_extent_t<io_stat_t> fresh_inline_by_ext;
+    counter_by_extent_t<io_stat_t> fresh_ool_by_ext;
     uint64_t num_trans = 0; // the number of inline records
     uint64_t num_ool_records = 0;
     uint64_t ool_record_padding_bytes = 0;
@@ -847,7 +827,7 @@ private:
   };
 
   struct success_read_trans_efforts_t {
-    effort_t read;
+    io_stat_t read;
     uint64_t num_trans = 0;
   };
 
diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
index 92971b44df6..5f1ee1d31fa 100644
--- a/src/crimson/os/seastore/transaction.h
+++ b/src/crimson/os/seastore/transaction.h
@@ -19,6 +19,28 @@ namespace crimson::os::seastore {
 class SeaStore;
 class Transaction;
 
+struct io_stat_t {
+  uint64_t num = 0;
+  uint64_t bytes = 0;
+
+  bool is_clear() const {
+    return (num == 0 && bytes == 0);
+  }
+
+  void increment(uint64_t _bytes) {
+    ++num;
+    bytes += _bytes;
+  }
+
+  void increment_stat(const io_stat_t& stat) {
+    num += stat.num;
+    bytes += stat.bytes;
+  }
+};
+inline std::ostream& operator<<(std::ostream& out, const io_stat_t& stat) {
+  return out << stat.num << "(" << stat.bytes << "B)";
+}
+
 /**
  * Transaction
  *
@@ -100,8 +122,7 @@ public:
       offset += ref->get_length();
       inline_block_list.push_back(ref);
     }
-    ++fresh_block_stats.num;
-    fresh_block_stats.bytes += ref->get_length();
+    fresh_block_stats.increment(ref->get_length());
     SUBTRACET(seastore_tm, "adding {} to write_set", *this, *ref);
     write_set.insert(*ref);
   }
@@ -188,14 +209,6 @@ public:
     std::for_each(inline_block_list.begin(), inline_block_list.end(), f);
   }
 
-  struct io_stat_t {
-    uint64_t num = 0;
-    uint64_t bytes = 0;
-
-    bool is_clear() const {
-      return (num == 0 && bytes == 0);
-    }
-  };
   const io_stat_t& get_fresh_block_stats() const {
     return fresh_block_stats;
   }