os/bluestore: optimize SharedBlob::sbid in-mem representation.

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 6b6263afe10ba492f587978eff67bf83b6db2f82..54db484ac341e95a8cf6089125c84a4a60259be5 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -1367,19 +1367,19 @@ bool BlueStore::OnodeSpace::map_any(std::function<bool(OnodeRef)> f)
 ostream& operator<<(ostream& out, const BlueStore::SharedBlob& sb)
 {
   out << "SharedBlob(" << &sb;
-  if (sb.sbid) {
-    out << " sbid 0x" << std::hex << sb.sbid << std::dec;
-  }
-  if (sb.persistent) {
+  
+  if (sb.loaded) {
     out << " loaded " << *sb.persistent;
+  } else {
+    out << " sbid 0x" << std::hex << sb.sbid_unloaded << std::dec;
   }
   return out << ")";
 }
 
 BlueStore::SharedBlob::SharedBlob(uint64_t i, Collection *_coll)
-  : sbid(i), coll(_coll), persistent(nullptr)
+  : coll(_coll), sbid_unloaded(i)
 {
-  assert(sbid > 0);
+  assert(sbid_unloaded > 0);
   if (get_cache()) {
     get_cache()->add_blob();
   }
@@ -1392,7 +1392,9 @@ BlueStore::SharedBlob::~SharedBlob()
     bc._clear(get_cache());
     get_cache()->rm_blob();
   }
-  delete persistent; //nullptr is OK to delete too
+  if (loaded && persistent) {
+    delete persistent; 
+  }
 }
 
 void BlueStore::SharedBlob::put()
@@ -2026,7 +2028,7 @@ bool BlueStore::ExtentMap::encode_some(
     p->blob->bound_encode(
       bound,
       struct_v,
-      p->blob->shared_blob->sbid,
+      p->blob->shared_blob->get_sbid(),
       false);
   }
 
@@ -2079,7 +2081,7 @@ bool BlueStore::ExtentMap::encode_some(
       }
       pos = p->logical_end();
       if (include_blob) {
-       p->blob->encode(app, struct_v, p->blob->shared_blob->sbid, false);
+       p->blob->encode(app, struct_v, p->blob->shared_blob->get_sbid(), false);
       }
     }
   }
@@ -2177,7 +2179,7 @@ void BlueStore::ExtentMap::bound_encode_spanning_blobs(size_t& p)
   denc_varint((uint32_t)0, key_size);
   p += spanning_blob_map.size() * key_size;
   for (const auto& i : spanning_blob_map) {
-    i.second->bound_encode(p, struct_v, i.second->shared_blob->sbid, true);
+    i.second->bound_encode(p, struct_v, i.second->shared_blob->get_sbid(), true);
   }
 }
 
@@ -2193,7 +2195,7 @@ void BlueStore::ExtentMap::encode_spanning_blobs(
   denc_varint(spanning_blob_map.size(), p);
   for (auto& i : spanning_blob_map) {
     denc_varint(i.second->id, p);
-    i.second->encode(p, struct_v, i.second->shared_blob->sbid, true);
+    i.second->encode(p, struct_v, i.second->shared_blob->get_sbid(), true);
   }
 }
 
@@ -2587,26 +2589,28 @@ void BlueStore::Collection::load_shared_blob(SharedBlobRef sb)
 
     bufferlist v;
     string key;
-    get_shared_blob_key(sb->sbid, &key);
+    auto sbid = sb->get_sbid();
+    get_shared_blob_key(sbid, &key);
     int r = store->db->get(PREFIX_SHARED_BLOB, key, &v);
     if (r < 0) {
-       lderr(store->cct) << __func__ << " sbid 0x" << std::hex << sb->sbid
+       lderr(store->cct) << __func__ << " sbid 0x" << std::hex << sbid
                          << std::dec << " not found at key "
                          << pretty_binary_string(key) << dendl;
       assert(0 == "uh oh, missing shared_blob");
     }
 
-    sb->persistent = new bluestore_shared_blob_t();
+    sb->loaded = true;
+    sb->persistent = new bluestore_shared_blob_t(sbid);
     bufferlist::iterator p = v.begin();
     ::decode(*(sb->persistent), p);
-    ldout(store->cct, 10) << __func__ << " sbid 0x" << std::hex << sb->sbid
+    ldout(store->cct, 10) << __func__ << " sbid 0x" << std::hex << sbid
                          << std::dec << " loaded shared_blob " << *sb << dendl;
   }
 }
 
 void BlueStore::Collection::make_blob_shared(uint64_t sbid, BlobRef b)
 {
-  assert(b->shared_blob->is_loaded());
+  assert(!b->shared_blob->is_loaded());
 
   ldout(store->cct, 10) << __func__ << " " << *b << dendl;
   bluestore_blob_t& blob = b->dirty_blob();
@@ -2616,8 +2620,8 @@ void BlueStore::Collection::make_blob_shared(uint64_t sbid, BlobRef b)
   blob.clear_flag(bluestore_blob_t::FLAG_MUTABLE);
 
   // update shared blob
-  b->shared_blob->sbid = sbid;
-  b->shared_blob->persistent = new bluestore_shared_blob_t();
+  b->shared_blob->loaded = true;
+  b->shared_blob->persistent = new bluestore_shared_blob_t(sbid);
   shared_blob_set.add(this, b->shared_blob.get());
   for (auto p : blob.extents) {
     if (p.is_valid()) {
@@ -4768,18 +4772,18 @@ int BlueStore::fsck(bool deep)
            i.first->get_referenced_bytes();
        }
        if (blob.is_shared()) {
-         if (i.first->shared_blob->sbid > blobid_max) {
+         if (i.first->shared_blob->get_sbid() > blobid_max) {
            derr << __func__ << " " << oid << " blob " << blob
-                << " sbid " << i.first->shared_blob->sbid << " > blobid_max "
+                << " sbid " << i.first->shared_blob->get_sbid() << " > blobid_max "
                 << blobid_max << dendl;
            ++errors;
-         } else if (i.first->shared_blob->sbid == 0) {
+         } else if (i.first->shared_blob->get_sbid() == 0) {
             derr << __func__ << " " << oid << " blob " << blob
                  << " marked as shared but has uninitialized sbid"
                  << dendl;
             ++errors;
           }
-         sb_info_t& sbi = sb_info[i.first->shared_blob->sbid];
+         sb_info_t& sbi = sb_info[i.first->shared_blob->get_sbid()];
          sbi.sb = i.first->shared_blob;
          sbi.oids.push_back(oid);
          sbi.compressed = blob.is_compressed();
@@ -4836,7 +4840,7 @@ int BlueStore::fsck(bool deep)
       } else {
        ++num_shared_blobs;
        sb_info_t& sbi = p->second;
-       bluestore_shared_blob_t shared_blob;
+       bluestore_shared_blob_t shared_blob(sbid);
        bufferlist bl = it->value();
        bufferlist::iterator blp = bl.begin();
        ::decode(shared_blob, blp);
@@ -6775,15 +6779,16 @@ void BlueStore::_txc_write_nodes(TransContext *txc, KeyValueDB::Transaction t)
   // finalize shared_blobs
   for (auto sb : txc->shared_blobs) {
     string key;
-    get_shared_blob_key(sb->sbid, &key);
+    auto sbid = sb->get_sbid();
+    get_shared_blob_key(sbid, &key);
     if (sb->persistent->empty()) {
-      dout(20) << "  shared_blob 0x" << std::hex << sb->sbid << std::dec
+      dout(20) << "  shared_blob 0x" << std::hex << sbid << std::dec
               << " is empty" << dendl;
       t->rmkey(PREFIX_SHARED_BLOB, key);
     } else {
       bufferlist bl;
       ::encode(*(sb->persistent), bl);
-      dout(20) << "  shared_blob 0x" << std::hex << sb->sbid << std::dec
+      dout(20) << "  shared_blob 0x" << std::hex << sbid << std::dec
               << " is " << bl.length() << dendl;
       t->set(PREFIX_SHARED_BLOB, key, bl);
     }

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index aff2f54131856b0f2fb607d0db0be2c51f6d78c4..d6c57869a672a5ce5ee83177d820785ec9c112e3 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -323,14 +323,16 @@ public:
     MEMPOOL_CLASS_HELPERS();
 
     std::atomic_int nref = {0}; ///< reference count
+    bool loaded = false;
 
-    // these are defined/set if the blob is marked 'shared'
-    uint64_t sbid = 0;          ///< shared blob id
     Collection *coll = nullptr;
-    bluestore_shared_blob_t *persistent; ///< persistent part of the shared blob if any
+    union {
+      uint64_t sbid_unloaded;              ///< sbid if persistent isn't loaded
+      bluestore_shared_blob_t *persistent; ///< persistent part of the shared blob if any
+    };
     BufferSpace bc;             ///< buffer cache
 
-    SharedBlob(Collection *_coll) : coll(_coll), persistent(nullptr) {
+    SharedBlob(Collection *_coll) : coll(_coll), sbid_unloaded(0) {
       if (get_cache()) {
        get_cache()->add_blob();
       }
@@ -338,6 +340,10 @@ public:
     SharedBlob(uint64_t i, Collection *_coll);
     ~SharedBlob();
 
+    uint64_t get_sbid() const {
+      return loaded ? persistent->sbid : sbid_unloaded;
+    }
+
     friend void intrusive_ptr_add_ref(SharedBlob *b) { b->get(); }
     friend void intrusive_ptr_release(SharedBlob *b) { b->put(); }
 
@@ -356,11 +362,11 @@ public:
       PExtentVector *r);
 
     friend bool operator==(const SharedBlob &l, const SharedBlob &r) {
-      return l.sbid == r.sbid;
+      return l.get_sbid() == r.get_sbid();
     }
     friend std::size_t hash_value(const SharedBlob &e) {
       rjhash<uint32_t> h;
-      return h(e.sbid);
+      return h(e.get_sbid());
     }
     inline Cache* get_cache() {
       return coll ? coll->cache : nullptr;
@@ -369,7 +375,7 @@ public:
       return coll ? &(coll->shared_blob_set) : nullptr;
     }
     inline bool is_loaded() const {
-      return persistent != nullptr;
+      return loaded;
     }
 
   };
@@ -394,7 +400,7 @@ public:
 
     void add(Collection* coll, SharedBlob *sb) {
       std::lock_guard<std::mutex> l(lock);
-      sb_map[sb->sbid] = sb;
+      sb_map[sb->get_sbid()] = sb;
       sb->coll = coll;
     }
 
@@ -402,7 +408,7 @@ public:
       std::lock_guard<std::mutex> l(lock);
       if (sb->nref == 0) {
        assert(sb->get_parent() == this);
-       sb_map.erase(sb->sbid);
+       sb_map.erase(sb->get_sbid());
        return true;
       }
       return false;

diff --git a/src/os/bluestore/bluestore_types.cc b/src/os/bluestore/bluestore_types.cc
index 4da726faa1ba744d010c102493403225fabdee95..6103612a72d2a8c6b6188e9e7fc968397a7d83b5 100644 (file)
--- a/src/os/bluestore/bluestore_types.cc
+++ b/src/os/bluestore/bluestore_types.cc
@@ -732,18 +732,20 @@ int bluestore_blob_t::verify_csum(uint64_t b_off, const bufferlist& bl,
 
 void bluestore_shared_blob_t::dump(Formatter *f) const
 {
+  f->dump_int("sbid", sbid);
   f->dump_object("ref_map", ref_map);
 }
 
 void bluestore_shared_blob_t::generate_test_instances(
   list<bluestore_shared_blob_t*>& ls)
 {
-  ls.push_back(new bluestore_shared_blob_t);
+  ls.push_back(new bluestore_shared_blob_t(1));
 }
 
 ostream& operator<<(ostream& out, const bluestore_shared_blob_t& sb)
 {
-  out << "shared_blob(" << sb.ref_map << ")";
+  out << " sbid 0x" << std::hex << sb.sbid << std::dec;
+  out << " ref_map(" << sb.ref_map << ")";
   return out;
 }
 

diff --git a/src/os/bluestore/bluestore_types.h b/src/os/bluestore/bluestore_types.h
index 610f1fe29b2beb55d3b075c703c10e2c34ace729..7b14b87e4cb656f914251bfb3084f53809e69950 100644 (file)
--- a/src/os/bluestore/bluestore_types.h
+++ b/src/os/bluestore/bluestore_types.h
@@ -804,14 +804,18 @@ ostream& operator<<(ostream& out, const bluestore_blob_t& o);
 
 /// shared blob state
 struct bluestore_shared_blob_t {
+  uint64_t sbid;                       ///> shared blob id
   bluestore_extent_ref_map_t ref_map;  ///< shared blob extents
 
+  bluestore_shared_blob_t(uint64_t _sbid) : sbid(_sbid) {}
+
   DENC(bluestore_shared_blob_t, v, p) {
     DENC_START(1, 1, p);
     denc(v.ref_map, p);
     DENC_FINISH(p);
   }
 
+
   void dump(Formatter *f) const;
   static void generate_test_instances(list<bluestore_shared_blob_t*>& ls);