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 79dcffd0ad5d245c6e8e932f9dbc6e80bc844771..de8b2f34f022df4b47d0cff360b9d034202974dc 100644 (file)
--- 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 ffbd2f6a8329ccbc39fc83ecf56cd4566e729047..f4ae5a744ed7aefc21a8ceb33c4329c74a5d0bb3 100644 (file)
--- 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 d1686d92dbe6de37b50ecbe0729a8ced5b2a8a10..a343443cda09f4d8d1c0366212c80e7ef2ab56fd 100644 (file)
--- 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 cbab5919cdd8642a30e81a6b17fbc34d284529ef..b1af1b6c4c8aa1ff45fdd67548bc55759fcef0c7 100644 (file)
--- 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');