denc: do not always copy before decoding
authorKefu Chai <kchai@redhat.com>
Mon, 26 Nov 2018 16:00:43 +0000 (00:00 +0800)
committerKefu Chai <kchai@redhat.com>
Tue, 27 Nov 2018 16:26:02 +0000 (00:26 +0800)
before this change, if the caller calls into ::decode_nohead(),
we will try to copy the memory chunk to be decoded before decoding it,
but if the buffer in the buffer list is not the last one, we will *deep*
copy the remaining part in the buffer list into a new contiguous memory
chunk and decode it instead. if we are decoding a map<int, buffer::ptr>
with lots of items in it, and the buffer::ptrs in it  are very small,
we will end up suffering from a serious internal fragmentation.

we could use the same strategy of decode(), where we compare the
size of remaining part with CEPH_PAGE_SIZE, and only copy it if
it's small enough. this requires that the decoded type supports
both variant of decoding contiguous and non-contiguous.
quite a few MDS types do not support both of them. for instance,
snapid_t only supports contiguous decoding.

so, instead of conditioning on size, in this change, we condition
on the traits::need_contiguous. probably we can condition on both
of them in a follow-up change.

Signed-off-by: Kefu Chai <kchai@redhat.com>
src/include/denc.h

index d56bf93d86132cddc2310a2429bc7665b87ab7a9..de74e41faad789ae290b5c2aa110783bf5e17cb3 100644 (file)
@@ -1585,12 +1585,16 @@ inline std::enable_if_t<traits::supported && !traits::featured> decode_nohead(
     return;
   if (p.end())
     throw buffer::end_of_buffer();
-  bufferptr tmp;
-  auto t = p;
-  t.copy_shallow(p.get_bl().length() - p.get_off(), tmp);
-  auto cp = std::cbegin(tmp);
-  traits::decode_nohead(num, o, cp);
-  p.advance(cp.get_offset());
+  if constexpr (traits::need_contiguous) {
+    bufferptr tmp;
+    auto t = p;
+    t.copy_shallow(p.get_bl().length() - p.get_off(), tmp);
+    auto cp = std::cbegin(tmp);
+    traits::decode_nohead(num, o, cp);
+    p.advance(cp.get_offset());
+  } else {
+    traits::decode_nohead(num, o, p);
+  }
 }
 }