os: get rid of the Transaction::decode_bp()

`os::Transaction::decode_bp()` has only one user: `_setattrs()`
of `BlueStore`. It uses that for optimization purposes: keeping
up contigous space instead of potentially fragmented `bufferlist`
that would require rectifying memcpy later.
The problem is `_setattrs()` also needs to avoid keeping large
raw buffers with only small subset being referenced. It achieves
this by copying the data if `bufferptr:::is_partial()` returns
`true`. However, this means the memcpy happens virtually always
as it's hard to even imagine the `val`, decoded from the wire,
can fulfill the 0 waste requirement.
Therefore the optimization doesn't make sense; it only imposes
costs in terms of complexity breaking the symmetry between encode
and decode in `os::Transation` (there is no `encode_bp()`).

This commit kills the optimization and simplifies `os::Transaction`.

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

diff --git a/src/os/Transaction.h b/src/os/Transaction.h
index f349a29c9f1203a88e93f350d498d0a0e21e7899..19896c79f95e29f1171256015d1515687eb7b332 100644 (file)
--- a/src/os/Transaction.h
+++ b/src/os/Transaction.h
@@ -711,10 +711,6 @@ public:
       decode(s, data_bl_p);
       return s;
     }
-    void decode_bp(ceph::buffer::ptr& bp) {
-       using ceph::decode;
-      decode(bp, data_bl_p);
-    }
     void decode_bl(ceph::buffer::list& bl) {
        using ceph::decode;
       decode(bl, data_bl_p);

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 44a171873c08cdb4ca8b6e7478df074fccaa9b37..791d03559db8aad3bde6fbee759c53ecf062aa6c 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -15690,9 +15690,9 @@ void BlueStore::_txc_add_transaction(TransContext *txc, Transaction *t)
     case Transaction::OP_SETATTR:
       {
         string name = i.decode_string();
-        bufferptr bp;
-        i.decode_bp(bp);
-       r = _setattr(txc, c, o, name, bp);
+        bufferlist bl;
+        i.decode_bl(bl);
+       r = _setattr(txc, c, o, name, bl);
       }
       break;
 
@@ -17733,18 +17733,22 @@ int BlueStore::_setattr(TransContext *txc,
                        CollectionRef& c,
                        OnodeRef& o,
                        const string& name,
-                       bufferptr& val)
+                       bufferlist& val)
 {
   dout(15) << __func__ << " " << c->cid << " " << o->oid
           << " " << name << " (" << val.length() << " bytes)"
           << dendl;
   int r = 0;
-  if (val.is_partial()) {
-    auto& b = o->onode.attrs[name.c_str()] = bufferptr(val.c_str(),
-                                                      val.length());
+  if (!val.length()) {
+    auto& b = o->onode.attrs[name.c_str()] = bufferptr("", 0);
     b.reassign_to_mempool(mempool::mempool_bluestore_cache_meta);
-  } else {
-    auto& b = o->onode.attrs[name.c_str()] = val;
+  } else if (!val.is_contiguous()) {
+    val.rebuild();
+    auto& b = o->onode.attrs[name.c_str()] = val.front();
+    b.reassign_to_mempool(mempool::mempool_bluestore_cache_meta);
+  } else if (val.front().is_partial()) {
+    val.rebuild();
+    auto& b = o->onode.attrs[name.c_str()] = val.front();
     b.reassign_to_mempool(mempool::mempool_bluestore_cache_meta);
   }
   txc->write_onode(o);

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 207ae2ec7a255590f60ab74504f7a0ae42558894..c5fc66fd16cb602d1272f8a1d9ae90193004e788 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -3862,7 +3862,7 @@ private:
               CollectionRef& c,
               OnodeRef& o,
               const std::string& name,
-              ceph::buffer::ptr& val);
+              ceph::buffer::list& val);
   int _setattrs(TransContext *txc,
                CollectionRef& c,
                OnodeRef& o,