crimson/onode-staged-tree: cleanup, decouple test_item_t from TestValue

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

src/crimson/os/seastore/onode_manager/staged-fltree/tree_utils.h

@@ -26,44 +26,50 @@
 namespace crimson::os::seastore::onode {
 
 /**
- * ValueItem template to work with tree utility classes:
+ * templates to work with tree utility classes:
  *
  * struct ValueItem {
- *   using ValueType = ConcreteValueType;
  *   <public members>
  *
  *   value_size_t get_payload_size() const;
- *   void initialize(Transaction& t, ValueType& value) const;
- *   void validate(ValueType& value) const;
  *   static ValueItem create(std::size_t expected_size, std::size_t id);
  * };
  * std::ostream& operator<<(std::ostream& os, const ValueItem& item);
+ *
+ * class ValueImpl final : public Value {
+ *   ...
+ *
+ *   using item_t = ValueItem;
+ *   void initialize(Transaction& t, const item_t& item);
+ *   void validate(const item_t& item);
+ * };
+ *
  */
 
-template <typename ValueItem>
+template <typename CursorType>
 void initialize_cursor_from_item(
     Transaction& t,
     const ghobject_t& key,
-    const ValueItem& item,
+    const typename decltype(std::declval<CursorType>().value())::item_t& item,
-    typename Btree<typename ValueItem::ValueType>::Cursor& cursor,
+    CursorType& cursor,
     bool insert_success) {
   ceph_assert(insert_success);
   ceph_assert(!cursor.is_end());
   ceph_assert(cursor.get_ghobj() == key);
   auto tree_value = cursor.value();
-  item.initialize(t, tree_value);
+  tree_value.initialize(t, item);
 }
 
 
-template <typename ValueItem>
+template <typename CursorType>
 void validate_cursor_from_item(
     const ghobject_t& key,
-    const ValueItem& item,
+    const typename decltype(std::declval<CursorType>().value())::item_t& item,
-    typename Btree<typename ValueItem::ValueType>::Cursor& cursor) {
+    CursorType& cursor) {
   ceph_assert(!cursor.is_end());
   ceph_assert(cursor.get_ghobj() == key);
-  auto value = cursor.value();
+  auto tree_value = cursor.value();
-  item.validate(value);
+  tree_value.validate(item);
 }
 
 template <typename ValueItem>
@@ -256,11 +262,12 @@ class KVPool {
   kvptr_vector_t random_p_kvs;
 };
 
-template <bool TRACK, typename ValueItem>
+template <bool TRACK, typename ValueImpl>
 class TreeBuilder {
  public:
-  using BtreeImpl = Btree<typename ValueItem::ValueType>;
+  using BtreeImpl = Btree<ValueImpl>;
   using BtreeCursor = typename BtreeImpl::Cursor;
+  using ValueItem = typename ValueImpl::item_t;
   using iterator_t = typename KVPool<ValueItem>::iterator_t;
 
   TreeBuilder(KVPool<ValueItem>& kvs, NodeExtentManagerURef&& nm)

src/test/crimson/seastore/onode_tree/test_staged_fltree.cc

@@ -1526,7 +1526,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase)
     auto moved_nm = (TEST_SEASTORE ? NodeExtentManager::create_seastore(*tm)
                                    : NodeExtentManager::create_dummy(IS_DUMMY_SYNC));
     auto p_nm = moved_nm.get();
-    auto tree = std::make_unique<TreeBuilder<TRACK_CURSORS, test_item_t>>(
+    auto tree = std::make_unique<TreeBuilder<TRACK_CURSORS, TestValue>>(
         kvs, std::move(moved_nm));
     {
       auto t = tm->create_transaction();
@@ -1623,7 +1623,7 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain)
     auto moved_nm = NodeExtentManager::create_seastore(
         *tm, L_ADDR_MIN, EAGAIN_PROBABILITY);
     auto p_nm = static_cast<SeastoreNodeExtentManager<true>*>(moved_nm.get());
-    auto tree = std::make_unique<TreeBuilder<TRACK_CURSORS, test_item_t>>(
+    auto tree = std::make_unique<TreeBuilder<TRACK_CURSORS, TestValue>>(
         kvs, std::move(moved_nm));
     unsigned num_ops = 0;
     unsigned num_ops_eagain = 0;

src/test/crimson/seastore/onode_tree/test_value.h

@@ -8,11 +8,39 @@
 
 namespace crimson::os::seastore::onode {
 
+struct test_item_t {
+  using id_t = uint16_t;
+  using magic_t = uint32_t;
+
+  value_size_t size;
+  id_t id;
+  magic_t magic;
+
+  value_size_t get_payload_size() const {
+    assert(size > sizeof(value_header_t));
+    return static_cast<value_size_t>(size - sizeof(value_header_t));
+  }
+
+  static test_item_t create(std::size_t _size, std::size_t _id) {
+    ceph_assert(_size <= std::numeric_limits<value_size_t>::max());
+    ceph_assert(_size > sizeof(value_header_t));
+    value_size_t size = _size;
+
+    ceph_assert(_id <= std::numeric_limits<id_t>::max());
+    id_t id = _id;
+
+    return {size, id, (magic_t)id * 137};
+  }
+};
+inline std::ostream& operator<<(std::ostream& os, const test_item_t& item) {
+  return os << "TestItem(#" << item.id << ", " << item.size << "B)";
+}
+
 class TestValue final : public Value {
  public:
   static constexpr auto HEADER_MAGIC = value_magic_t::TEST;
-  using id_t = uint16_t;
+  using id_t = test_item_t::id_t;
-  using magic_t = uint32_t;
+  using magic_t = test_item_t::magic_t;
   struct magic_packed_t {
     magic_t value;
   } __attribute__((packed));
@@ -138,45 +166,24 @@ class TestValue final : public Value {
     }
     Replayable::set_tail_magic(value_mutable.first, magic);
   }
-};
 
-struct test_item_t {
+  /*
-  using ValueType = TestValue;
+   * tree_util.h related interfaces
+   */
 
-  value_size_t size;
+  using item_t = test_item_t;
-  TestValue::id_t id;
-  TestValue::magic_t magic;
 
-  value_size_t get_payload_size() const {
+  void initialize(Transaction& t, const item_t& item) {
-    assert(size > sizeof(value_header_t));
+    ceph_assert(get_payload_size() + sizeof(value_header_t) == item.size);
-    return static_cast<value_size_t>(size - sizeof(value_header_t));
+    set_id_replayable(t, item.id);
+    set_tail_magic_replayable(t, item.magic);
   }
 
-  void initialize(Transaction& t, TestValue& value) const {
+  void validate(const item_t& item) const {
-    ceph_assert(value.get_payload_size() + sizeof(value_header_t) == size);
+    ceph_assert(get_payload_size() + sizeof(value_header_t) == item.size);
-    value.set_id_replayable(t, id);
+    ceph_assert(get_id() == item.id);
-    value.set_tail_magic_replayable(t, magic);
+    ceph_assert(get_tail_magic() == item.magic);
-  }
-
-  void validate(TestValue& value) const {
-    ceph_assert(value.get_payload_size() + sizeof(value_header_t) == size);
-    ceph_assert(value.get_id() == id);
-    ceph_assert(value.get_tail_magic() == magic);
-  }
-
-  static test_item_t create(std::size_t _size, std::size_t _id) {
-    ceph_assert(_size <= std::numeric_limits<value_size_t>::max());
-    ceph_assert(_size > sizeof(value_header_t));
-    value_size_t size = _size;
-
-    ceph_assert(_id <= std::numeric_limits<TestValue::id_t>::max());
-    TestValue::id_t id = _id;
-
-    return {size, id, (TestValue::magic_t)id * 137};
   }
 };
-inline std::ostream& operator<<(std::ostream& os, const test_item_t& item) {
-  return os << "TestItem(#" << item.id << ", " << item.size << "B)";
-}
 
 }

src/tools/crimson/perf_staged_fltree.cc

@@ -30,7 +30,7 @@ class PerfTree : public TMTestState {
   seastar::future<> run(KVPool<test_item_t>& kvs, double erase_ratio) {
     return tm_setup().then([this, &kvs, erase_ratio] {
       return seastar::async([this, &kvs, erase_ratio] {
-        auto tree = std::make_unique<TreeBuilder<TRACK, test_item_t>>(kvs,
+        auto tree = std::make_unique<TreeBuilder<TRACK, TestValue>>(kvs,
             (is_dummy ? NodeExtentManager::create_dummy(true)
                       : NodeExtentManager::create_seastore(*tm)));
         {