From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Fri, 15 Jan 2021 03:00:38 +0000 (+0800)
Subject: crimson/onode-staged-tree: encapsulate cached views to tree_cursor_t::Cache
X-Git-Tag: v17.1.0~3086^2~7
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f019b90f18f2f038ce4eeca809fac4fe41f061f9;p=ceph.git

crimson/onode-staged-tree: encapsulate cached views to tree_cursor_t::Cache

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

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
index 3df458f08623..7c5e5a34345a 100644
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
@@ -30,42 +30,32 @@ using node_future = Node::node_future<ValueT>;
  */
 
 tree_cursor_t::tree_cursor_t(Ref<LeafNode> node, const search_position_t& pos)
-      : leaf_node{node}, position{pos} {
+      : ref_leaf_node{node}, position{pos} {
   assert(!is_end());
-  leaf_node->do_track_cursor<true>(*this);
+  ref_leaf_node->do_track_cursor<true>(*this);
 }
 
 tree_cursor_t::tree_cursor_t(
     Ref<LeafNode> node, const search_position_t& pos,
-    const key_view_t& key, const onode_t* _p_value, layout_version_t v)
-      : leaf_node{node}, position{pos} {
+    const key_view_t& key_view, const onode_t* p_value)
+      : ref_leaf_node{node}, position{pos} {
   assert(!is_end());
-  update_kv(key, _p_value, v);
-  leaf_node->do_track_cursor<true>(*this);
+  update_cache(*node, key_view, p_value);
+  ref_leaf_node->do_track_cursor<true>(*this);
 }
 
 tree_cursor_t::tree_cursor_t(Ref<LeafNode> node)
-      : leaf_node{node}, position{search_position_t::end()} {
+      : ref_leaf_node{node}, position{search_position_t::end()} {
   assert(is_end());
-  assert(leaf_node->is_level_tail());
+  assert(ref_leaf_node->is_level_tail());
 }
 
 tree_cursor_t::~tree_cursor_t() {
   if (!is_end()) {
-    leaf_node->do_untrack_cursor(*this);
+    ref_leaf_node->do_untrack_cursor(*this);
   }
 }
 
-const key_view_t& tree_cursor_t::get_key_view() const {
-  ensure_kv();
-  return *key_view;
-}
-
-const onode_t* tree_cursor_t::get_p_value() const {
-  ensure_kv();
-  return p_value;
-}
-
 template <bool VALIDATE>
 void tree_cursor_t::update_track(
     Ref<LeafNode> node, const search_position_t& pos) {
@@ -73,32 +63,59 @@ void tree_cursor_t::update_track(
   // track the new node and new pos
   assert(!pos.is_end());
   assert(!is_end());
-  leaf_node = node;
+  ref_leaf_node = node;
   position = pos;
-  key_view.reset();
-  p_value = nullptr;
-  leaf_node->do_track_cursor<VALIDATE>(*this);
+  cache.invalidate();
+  ref_leaf_node->do_track_cursor<VALIDATE>(*this);
 }
 template void tree_cursor_t::update_track<true>(Ref<LeafNode>, const search_position_t&);
 template void tree_cursor_t::update_track<false>(Ref<LeafNode>, const search_position_t&);
 
-void tree_cursor_t::update_kv(
-    const key_view_t& key, const onode_t* _p_value, layout_version_t v) const {
+void tree_cursor_t::update_cache(LeafNode& node,
+                                 const key_view_t& key_view,
+                                 const onode_t* p_value) const {
   assert(!is_end());
-  assert(_p_value);
-  assert(std::make_tuple(key, _p_value, v) == leaf_node->get_kv(position));
-  key_view = key;
-  p_value = _p_value;
-  node_version = v;
+  assert(ref_leaf_node.get() == &node);
+  cache.update(node, key_view, p_value);
+  cache.validate_is_latest(node, position);
 }
 
-void tree_cursor_t::ensure_kv() const {
+void tree_cursor_t::maybe_update_cache() const {
   assert(!is_end());
-  if (!p_value || node_version != leaf_node->get_layout_version()) {
-    // NOTE: the leaf node is always present when we hold its reference.
-    std::tie(key_view, p_value, node_version) = leaf_node->get_kv(position);
+  if (!cache.is_latest()) {
+    auto [key_view, p_value] = ref_leaf_node->get_kv(position);
+    cache.update(*ref_leaf_node, key_view, p_value);
   }
-  assert(p_value);
+  cache.validate_is_latest(*ref_leaf_node, position);
+}
+
+tree_cursor_t::Cache::Cache() = default;
+
+bool tree_cursor_t::Cache::is_latest() const {
+  return (valid && (version == p_leaf_node->get_layout_version()));
+}
+
+void tree_cursor_t::Cache::update(LeafNode& node,
+                                  const key_view_t& _key_view,
+                                  const onode_t* _p_value) {
+  assert(_p_value);
+  p_leaf_node = &node;
+  version = node.get_layout_version();
+  key_view = _key_view;
+  p_value = _p_value;
+  valid = true;
+  assert(is_latest());
+}
+
+void tree_cursor_t::Cache::validate_is_latest(const LeafNode& node,
+                                              const search_position_t& pos) const {
+  assert(p_leaf_node == &node);
+  assert(is_latest());
+#ifndef NDEBUG
+  auto [_key_view, _p_value] = node.get_kv(pos);
+  assert(*key_view == _key_view);
+  assert(p_value == _p_value);
+#endif
 }
 
 /*
@@ -573,11 +590,11 @@ bool LeafNode::is_level_tail() const {
   return impl->is_level_tail();
 }
 
-std::tuple<key_view_t, const onode_t*, layout_version_t> LeafNode::get_kv(
-    const search_position_t& pos) const {
+std::tuple<key_view_t, const onode_t*>
+LeafNode::get_kv(const search_position_t& pos) const {
   key_view_t key_view;
   auto p_value = impl->get_p_value(pos, &key_view);
-  return {key_view, p_value, layout_version};
+  return {key_view, p_value};
 }
 
 node_future<Ref<tree_cursor_t>>
@@ -737,12 +754,12 @@ Ref<tree_cursor_t> LeafNode::get_or_track_cursor(
   Ref<tree_cursor_t> p_cursor;
   auto found = tracked_cursors.find(position);
   if (found == tracked_cursors.end()) {
-    p_cursor = new tree_cursor_t(this, position, key, p_value, layout_version);
+    p_cursor = new tree_cursor_t(this, position, key, p_value);
   } else {
     p_cursor = found->second;
     assert(p_cursor->get_leaf_node() == this);
     assert(p_cursor->get_position() == position);
-    p_cursor->update_kv(key, p_value, layout_version);
+    p_cursor->update_cache(*this, key, p_value);
   }
   return p_cursor;
 }
@@ -751,9 +768,9 @@ void LeafNode::validate_cursor(tree_cursor_t& cursor) const {
 #ifndef NDEBUG
   assert(this == cursor.get_leaf_node().get());
   assert(!cursor.is_end());
-  auto [key, val, ver] = get_kv(cursor.get_position());
+  auto [key, p_value] = get_kv(cursor.get_position());
   assert(key == cursor.get_key_view());
-  assert(val == cursor.get_p_value());
+  assert(p_value == cursor.get_p_value());
 #endif
 }
 
diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node.h
index d6af489e7701..f508ff638517 100644
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.h
@@ -62,13 +62,14 @@ class tree_cursor_t final
   : public boost::intrusive_ref_counter<
            tree_cursor_t, boost::thread_unsafe_counter> {
  public:
-  // public to Btree
   ~tree_cursor_t();
   tree_cursor_t(const tree_cursor_t&) = delete;
   tree_cursor_t(tree_cursor_t&&) = delete;
   tree_cursor_t& operator=(const tree_cursor_t&) = delete;
   tree_cursor_t& operator=(tree_cursor_t&&) = delete;
 
+  // public to Btree
+
   /**
    * is_end
    *
@@ -79,38 +80,72 @@ class tree_cursor_t final
   bool is_end() const { return position.is_end(); }
 
   /// Returns the key view in tree if it is not an end cursor.
-  const key_view_t& get_key_view() const;
+  const key_view_t& get_key_view() const {
+    maybe_update_cache();
+    return cache.get_key_view();
+  }
 
   /// Returns the value pointer in tree if it is not an end cursor.
-  const onode_t* get_p_value() const;
+  const onode_t* get_p_value() const {
+    maybe_update_cache();
+    return cache.get_p_value();
+  }
 
  private:
   tree_cursor_t(Ref<LeafNode>, const search_position_t&);
   tree_cursor_t(Ref<LeafNode>, const search_position_t&,
-                const key_view_t& key, const onode_t*, layout_version_t);
+                const key_view_t&, const onode_t*);
   // lookup reaches the end, contain leaf node for further insert
   tree_cursor_t(Ref<LeafNode>);
+
   const search_position_t& get_position() const { return position; }
-  Ref<LeafNode> get_leaf_node() { return leaf_node; }
+  Ref<LeafNode> get_leaf_node() { return ref_leaf_node; }
   template <bool VALIDATE>
   void update_track(Ref<LeafNode>, const search_position_t&);
-  void update_kv(const key_view_t&, const onode_t*, layout_version_t) const;
-  void ensure_kv() const;
+  void update_cache(LeafNode&, const key_view_t&, const onode_t*) const;
+  void maybe_update_cache() const;
 
- private:
   /**
    * Reversed resource management (tree_cursor_t)
    *
    * tree_cursor_t holds a reference to the LeafNode, so the LeafNode will be
    * alive as long as any of it's cursors is still referenced by user.
    */
-  Ref<LeafNode> leaf_node;
+  Ref<LeafNode> ref_leaf_node;
   search_position_t position;
 
-  // cached information
-  mutable std::optional<key_view_t> key_view;
-  mutable const onode_t* p_value;
-  mutable layout_version_t node_version;
+  /** Cache
+   *
+   * Cached memory pointers or views which may be outdated due to
+   * asynchronous leaf node updates.
+   */
+  class Cache {
+   public:
+    Cache();
+    bool is_latest() const;
+    void invalidate() { valid = false; }
+    void update(LeafNode&, const key_view_t&, const onode_t*);
+    void validate_is_latest(const LeafNode&, const search_position_t&) const;
+
+    const key_view_t& get_key_view() const {
+      assert(is_latest());
+      assert(key_view.has_value());
+      return *key_view;
+    }
+    const onode_t* get_p_value() const {
+      assert(is_latest());
+      assert(p_value);
+      return p_value;
+    }
+
+   private:
+    LeafNode* p_leaf_node = nullptr;
+    std::optional<key_view_t> key_view;
+    const onode_t* p_value = nullptr;
+    layout_version_t version;
+    bool valid = false;
+  };
+  mutable Cache cache;
 
   friend class LeafNode;
   friend class Node; // get_position(), get_leaf_node()
@@ -393,8 +428,8 @@ class LeafNode final : public Node {
 
   bool is_level_tail() const;
   layout_version_t get_layout_version() const { return layout_version; }
-  std::tuple<key_view_t, const onode_t*, layout_version_t> get_kv(
-      const search_position_t&) const;
+  std::tuple<key_view_t, const onode_t*> get_kv(const search_position_t&) const;
+
   template <bool VALIDATE>
   void do_track_cursor(tree_cursor_t& cursor) {
     if constexpr (VALIDATE) {