From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Thu, 20 Sep 2018 01:45:10 +0000 (+0200)
Subject: common: optimize hole appending in bufferlist.
X-Git-Tag: v14.1.0~1058^2~4
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=0624e64edf4f11e2847526d345a7f65836f03e5f;p=ceph.git

common: optimize hole appending in bufferlist.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
---

diff --git a/src/common/buffer.cc b/src/common/buffer.cc
index 79dcffd0ad5..de8b2f34f02 100644
--- a/src/common/buffer.cc
+++ b/src/common/buffer.cc
@@ -1674,6 +1674,26 @@ using namespace ceph;
     }
   }
 
+  buffer::list::iterator buffer::list::append_hole(const unsigned len)
+  {
+    if (unlikely(append_buffer.unused_tail_length() < len)) {
+      // make a new append_buffer.  fill out a complete page, factoring in
+      // the raw_combined overhead.
+      const size_t need = \
+	round_up_to(len, sizeof(size_t)) + sizeof(raw_combined);
+      const size_t alen = \
+	round_up_to(need, CEPH_BUFFER_ALLOC_UNIT) - sizeof(raw_combined);
+      append_buffer = raw_combined::create(alen, 0, get_mempool());
+      append_buffer.set_length(0);
+    }
+
+    append_buffer.set_length(append_buffer.length() + len);
+    append(append_buffer, append_buffer.length() - len, len);
+
+    const auto lastbuf = std::prev(std::end(_buffers));
+    return { this, length() - len, lastbuf, lastbuf->length() - len };
+  }
+
   void buffer::list::prepend_zero(unsigned len)
   {
     ptr bp(len);
diff --git a/src/include/buffer.h b/src/include/buffer.h
index ffbd2f6a832..f4ae5a744ed 100644
--- a/src/include/buffer.h
+++ b/src/include/buffer.h
@@ -913,6 +913,7 @@ namespace buffer CEPH_BUFFER_API {
     void append(const ptr& bp, unsigned off, unsigned len);
     void append(const list& bl);
     void append(std::istream& in);
+    iterator append_hole(unsigned len);
     void append_zero(unsigned len);
     void prepend_zero(unsigned len);
     
diff --git a/src/include/encoding.h b/src/include/encoding.h
index d1686d92dbe..a343443cda0 100644
--- a/src/include/encoding.h
+++ b/src/include/encoding.h
@@ -1204,30 +1204,16 @@ decode(std::array<T, N>& v, bufferlist::const_iterator& p)
  * @param compat oldest code version that can decode it
  * @param bl bufferlist to encode to
  *
- * NOTE: to not advance() backward, we snapshot the end of bufferlist
- * in a given moment, and then rely on copy_in()'s ability to rewind:
- *
- * void buffer::list::iterator::copy_in(...)
- * {
- *   // copy
- *   if (p == ls->end())
- *     seek(off);
- *
- *   // ...
- * }
- *
- * This can be quite costly, so next commit is expected to remedy.
  */
 #define ENCODE_START(v, compat, bl)			     \
   using ::ceph::encode;					     \
   __u8 struct_v = v, struct_compat = compat;		     \
-  encode(struct_v, (bl));				     \
-  ::ceph::buffer::list::iterator struct_compat_it = (bl).end();	\
-  encode(struct_compat, (bl));			     \
   ceph_le32 struct_len;				             \
-  struct_len = 0;                                            \
-  ::ceph::buffer::list::iterator struct_len_it = (bl).end(); \
-  encode(struct_len, (bl));				     \
+  encode(struct_v, (bl));				     \
+  ::ceph::buffer::list::iterator struct_compat_it =	     \
+    (bl).append_hole(sizeof(struct_compat));		     \
+  ::ceph::buffer::list::iterator struct_len_it =	     \
+    (bl).append_hole(sizeof(struct_len));		     \
   do {
 
 /**
@@ -1242,8 +1228,8 @@ decode(std::array<T, N>& v, bufferlist::const_iterator& p)
   struct_len_it.copy_in(4, (char *)&struct_len);			\
   if (new_struct_compat) {						\
     struct_compat = new_struct_compat;					\
-    struct_compat_it.copy_in(1, (char *)&struct_compat);		\
-  }
+  }									\
+  struct_compat_it.copy_in(1, (char *)&struct_compat);
 
 #define ENCODE_FINISH(bl) ENCODE_FINISH_NEW_COMPAT(bl, 0)
 
diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc
index cbab5919cdd..b1af1b6c4c8 100644
--- a/src/test/bufferlist.cc
+++ b/src/test/bufferlist.cc
@@ -2110,6 +2110,48 @@ TEST(BufferList, append) {
   }
 }
 
+TEST(BufferList, append_hole) {
+  {
+    bufferlist bl;
+    auto iter = bl.append_hole(1);
+    EXPECT_EQ((unsigned)1, bl.get_num_buffers());
+    EXPECT_EQ((unsigned)1, bl.length());
+
+    bl.append("BC", 2);
+    EXPECT_EQ((unsigned)1, bl.get_num_buffers());
+    EXPECT_EQ((unsigned)3, bl.length());
+
+    const char a = 'A';
+    EXPECT_EQ((unsigned)2, bl.length() - iter.get_off() - sizeof(a));
+    iter.copy_in((unsigned)1, &a);
+    EXPECT_EQ((unsigned)3, bl.length());
+
+    EXPECT_EQ(0, ::memcmp("ABC", bl.c_str(), 3));
+  }
+
+  {
+    bufferlist bl;
+    bl.append('A');
+    EXPECT_EQ((unsigned)1, bl.get_num_buffers());
+    EXPECT_EQ((unsigned)1, bl.length());
+
+    auto iter = bl.append_hole(1);
+    EXPECT_EQ((unsigned)1, bl.get_num_buffers());
+    EXPECT_EQ((unsigned)2, bl.length());
+
+    bl.append('C');
+    EXPECT_EQ((unsigned)1, bl.get_num_buffers());
+    EXPECT_EQ((unsigned)3, bl.length());
+
+    const char b = 'B';
+    EXPECT_EQ((unsigned)1, bl.length() - iter.get_off() - sizeof(b));
+    iter.copy_in((unsigned)1, &b);
+    EXPECT_EQ((unsigned)3, bl.length());
+
+    EXPECT_EQ(0, ::memcmp("ABC", bl.c_str(), 3));
+  }
+}
+
 TEST(BufferList, append_zero) {
   bufferlist bl;
   bl.append('A');