Merge pull request ceph#56353 from myoungwon/wip-apply-shallow-copy-rbm-overwrite

crimson/os/seastore: avoid new allocation when overwriting data in RBM for performance

Reviewed-by: Yingxin Cheng <[email protected]>

Author: cyx1231st

src/common/options/crimson.yaml.in

@@ -126,5 +126,5 @@ options:
 - name: seastore_data_delta_based_overwrite
   type: size
   level: dev
-  desc: overwrite the existing data block based on delta if the original size is smaller than the value, otherwise do overwrite based on remapping, set to 0 to enforce the remap-based overwrite.
+  desc: overwrite the existing data block based on delta if the overwrite size is equal to or less than the value, otherwise do overwrite based on remapping, set to 0 to enforce the remap-based overwrite.
   default: 0

src/crimson/os/seastore/cached_extent.h

@@ -547,11 +547,11 @@ class CachedExtent
   }
 
   /// Get ref to raw buffer
-  bufferptr &get_bptr() {
+  virtual bufferptr &get_bptr() {
     assert(ptr.has_value());
     return *ptr;
   }
-  const bufferptr &get_bptr() const {
+  virtual const bufferptr &get_bptr() const {
     assert(ptr.has_value());
     return *ptr;
   }
@@ -648,11 +648,6 @@ class CachedExtent
     return extent_index_hook.is_linked();
   }
 
-  /// set bufferptr
-  void set_bptr(ceph::bufferptr &&nptr) {
-    ptr = nptr;
-  }
-
   /// hook for intrusive ref list (mainly dirty or lru list)
   boost::intrusive::list_member_hook<> primary_ref_list_hook;
   using primary_ref_list_member_options = boost::intrusive::member_hook<
@@ -799,6 +794,11 @@ class CachedExtent
     poffset = offset;
   }
 
+  /// set bufferptr
+  void set_bptr(ceph::bufferptr &&nptr) {
+    ptr = nptr;
+  }
+
   /**
    * maybe_generate_relative
    *

src/crimson/os/seastore/object_data_handler.cc

@@ -297,6 +297,7 @@ overwrite_ops_t prepare_ops_list(
   interval_set<uint64_t> pre_alloc_addr_removed, pre_alloc_addr_remapped;
   if (delta_based_overwrite_max_extent_size) {
     for (auto &r : ops.to_remove) {
+      // TODO: Introduce LBAMapping::is_data_stable() to include EXIST_CLEAN extents
       if (r->is_stable() && !r->is_zero_reserved()) {
 	pre_alloc_addr_removed.insert(r->get_key(), r->get_length());
 

src/crimson/os/seastore/object_data_handler.h

@@ -30,22 +30,72 @@ struct block_delta_t {
   }
 };
 
+class overwrite_buf_t {
+public:
+  overwrite_buf_t() = default;
+  bool is_empty() const {
+    return changes.empty() && !has_cached_bptr();
+  }
+  bool has_cached_bptr() const {
+    return ptr.has_value();
+  }
+  void add(const block_delta_t &b) {
+    changes.push_back(b);
+  }
+  void apply_changes_to(bufferptr &b) const {
+    assert(!changes.empty());
+    for (auto p : changes) {
+      auto iter = p.bl.cbegin();
+      iter.copy(p.bl.length(), b.c_str() + p.offset);
+    }
+    changes.clear();
+  }
+  const bufferptr &get_cached_bptr(const bufferptr &_ptr) const {
+    apply_changes_to_cache(_ptr);
+    return *ptr;
+  }
+  bufferptr &get_cached_bptr(const bufferptr &_ptr) {
+    apply_changes_to_cache(_ptr);
+    return *ptr;
+  }
+  bufferptr &&move_cached_bptr() {
+    assert(has_cached_bptr());
+    apply_changes_to(*ptr);
+    return std::move(*ptr);
+  }
+private:
+  void apply_changes_to_cache(const bufferptr &_ptr) const {
+    assert(!is_empty());
+    if (!has_cached_bptr()) {
+      ptr = ceph::buffer::copy(_ptr.c_str(), _ptr.length());
+    }
+    if (!changes.empty()) {
+      apply_changes_to(*ptr);
+    }
+  }
+  mutable std::vector<block_delta_t> changes = {};
+  mutable std::optional<ceph::bufferptr> ptr = std::nullopt;
+};
+
 struct ObjectDataBlock : crimson::os::seastore::LogicalCachedExtent {
   using Ref = TCachedExtentRef<ObjectDataBlock>;
 
   std::vector<block_delta_t> delta = {};
 
   interval_set<extent_len_t> modified_region;
 
+  // to provide the local modified view during transaction
+  overwrite_buf_t cached_overwrites;
+
   explicit ObjectDataBlock(ceph::bufferptr &&ptr)
     : LogicalCachedExtent(std::move(ptr)) {}
-  explicit ObjectDataBlock(const ObjectDataBlock &other)
-    : LogicalCachedExtent(other), modified_region(other.modified_region) {}
+  explicit ObjectDataBlock(const ObjectDataBlock &other, share_buffer_t s)
+    : LogicalCachedExtent(other, s), modified_region(other.modified_region) {}
   explicit ObjectDataBlock(extent_len_t length)
     : LogicalCachedExtent(length) {}
 
   CachedExtentRef duplicate_for_write(Transaction&) final {
-    return CachedExtentRef(new ObjectDataBlock(*this));
+    return CachedExtentRef(new ObjectDataBlock(*this, share_buffer_t{}));
   };
 
   static constexpr extent_types_t TYPE = extent_types_t::OBJECT_DATA_BLOCK;
@@ -54,9 +104,9 @@ struct ObjectDataBlock : crimson::os::seastore::LogicalCachedExtent {
   }
 
   void overwrite(extent_len_t offset, bufferlist bl) {
-    auto iter = bl.cbegin();
-    iter.copy(bl.length(), get_bptr().c_str() + offset);
-    delta.push_back({offset, bl.length(), bl});
+    block_delta_t b {offset, bl.length(), bl};
+    cached_overwrites.add(b);
+    delta.push_back(b);
     modified_region.union_insert(offset, bl.length());
   }
 
@@ -76,9 +126,39 @@ struct ObjectDataBlock : crimson::os::seastore::LogicalCachedExtent {
     modified_region.clear();
   }
 
+  void prepare_commit() final {
+    if (is_mutation_pending() || is_exist_mutation_pending()) {
+      ceph_assert(!cached_overwrites.is_empty());
+      if (cached_overwrites.has_cached_bptr()) {
+        set_bptr(cached_overwrites.move_cached_bptr());
+      } else {
+        // The optimized path to minimize data copy
+        cached_overwrites.apply_changes_to(CachedExtent::get_bptr());
+      }
+    } else {
+      assert(cached_overwrites.is_empty());
+    }
+  }
+
   void logical_on_delta_write() final {
     delta.clear();
   }
+
+  bufferptr &get_bptr() override {
+    if (cached_overwrites.is_empty()) {
+      return CachedExtent::get_bptr();
+    } else {
+      return cached_overwrites.get_cached_bptr(CachedExtent::get_bptr());
+    }
+  }
+
+  const bufferptr &get_bptr() const override {
+    if (cached_overwrites.is_empty()) {
+      return CachedExtent::get_bptr();
+    } else {
+      return cached_overwrites.get_cached_bptr(CachedExtent::get_bptr());
+    }
+  }
 };
 using ObjectDataBlockRef = TCachedExtentRef<ObjectDataBlock>;