os/bluestore: switch spanning_blob_map to std::map

The intrusive map overhead on Blob is expensive when a tiny fraction of
Blobs actually end up in the spanning_blob_map.  Let's pay the extra
allocation overhead (for the map node holding the BlobRef) for those rare
cases.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index aaefd36c022562a5a0f8289b68fc01dacc7c2008..b827cef883c3cdc292324405f4b035d642977840 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -1574,11 +1574,9 @@ void BlueStore::ExtentMap::reshard(Onode *o, uint64_t min_alloc_size)
   // un-span all blobs
   auto p = spanning_blob_map.begin();
   while (p != spanning_blob_map.end()) {
-    auto n = spanning_blob_map.erase(p);
-    p->id = -1;
-    dout(30) << __func__ << " un-spanning " << *p << dendl;
-    p->put();
-    p = n;
+    p->second->id = -1;
+    dout(30) << __func__ << " un-spanning " << *p->second << dendl;
+    p = spanning_blob_map.erase(p);
   }
 
   if (extent_map.size() <= 1) {
@@ -1727,8 +1725,7 @@ void BlueStore::ExtentMap::reshard(Onode *o, uint64_t min_alloc_size)
        }
        if (must_span) {
          b->id = bid++;
-         spanning_blob_map.insert(*b);
-         b->get();
+         spanning_blob_map[b->id] = b;
          dout(20) << __func__ << "    adding spanning " << *b << dendl;
        }
       }
@@ -1886,10 +1883,10 @@ void BlueStore::ExtentMap::encode_spanning_blobs(bufferlist& bl)
 {
   unsigned n = spanning_blob_map.size();
   small_encode_varint(n, bl);
-  for (auto& b : spanning_blob_map) {
-    small_encode_varint(b.id, bl);
-    b.encode(bl);
-    b.ref_map.encode(bl);
+  for (auto& i : spanning_blob_map) {
+    small_encode_varint(i.second->id, bl);
+    i.second->encode(bl);
+    i.second->ref_map.encode(bl);
   }
 }
 
@@ -1902,24 +1899,13 @@ void BlueStore::ExtentMap::decode_spanning_blobs(
   while (n--) {
     BlobRef b(new Blob());
     small_decode_varint(b->id, p);
-    spanning_blob_map.insert(*b);
-    b->get();
+    spanning_blob_map[b->id] = b;
     b->decode(p);
     b->ref_map.decode(p);
     c->open_shared_blob(b);
   }
 }
 
-BlueStore::BlobRef BlueStore::ExtentMap::get_spanning_blob(
-  int id)
-{
-  Blob dummy;
-  dummy.id = id;
-  auto p = spanning_blob_map.find(dummy);
-  assert(p != spanning_blob_map.end());
-  return &*p;
-}
-
 void BlueStore::ExtentMap::init_shards(Onode *on, bool loaded, bool dirty)
 {
   shards.resize(on->onode.extent_map_shards.size());
@@ -7709,7 +7695,7 @@ void BlueStore::_wctx_finish(
     if (b->id >= 0 && b->ref_map.empty()) {
       dout(20) << __func__ << "  spanning_blob_map removing empty " << *b
               << dendl;
-      auto it = o->extent_map.spanning_blob_map.iterator_to(*b);
+      auto it = o->extent_map.spanning_blob_map.find(b->id);
       o->extent_map.spanning_blob_map.erase(it);
     }
   }
@@ -8504,8 +8490,7 @@ int BlueStore::_do_clone_range(
       id_to_blob[n] = cb;
       e.blob->dup(*cb);
       if (cb->id >= 0) {
-       newo->extent_map.spanning_blob_map.insert(*cb);
-       cb->get();
+       newo->extent_map.spanning_blob_map[cb->id] = cb;
       }
       // bump the extent refs on the copied blob's extents
       for (auto p : blob.extents) {

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index ccbf7587100ad782d4a1fa69271d52cc30a37ea7..6307c4604617856cb1ff23f7bc9ad4e87cd6082e 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -407,7 +407,7 @@ public:
   };
 
   /// in-memory blob metadata and associated cached buffers (if any)
-  struct Blob : public boost::intrusive::set_base_hook<boost::intrusive::optimize_size<true>> {
+  struct Blob {
     std::atomic_int nref = {0};     ///< reference count
     int id = -1;                    ///< id, for spanning blobs only, >= 0
     SharedBlobRef shared_blob;      ///< shared blob state (if any)
@@ -433,17 +433,6 @@ public:
 
     friend ostream& operator<<(ostream& out, const Blob &b);
 
-    // comparators for intrusive_set
-    friend bool operator<(const Blob &a, const Blob &b) {
-      return a.id < b.id;
-    }
-    friend bool operator>(const Blob &a, const Blob &b) {
-      return a.id > b.id;
-    }
-    friend bool operator==(const Blob &a, const Blob &b) {
-      return a.id == b.id;
-    }
-
     bool is_spanning() const {
       return id >= 0;
     }
@@ -521,7 +510,7 @@ public:
     }
   };
   typedef boost::intrusive_ptr<Blob> BlobRef;
-  typedef boost::intrusive::set<Blob> blob_map_t;
+  typedef std::map<int,BlobRef> blob_map_t;
 
   /// a logical extent, pointing to (some portion of) a blob
   struct Extent : public boost::intrusive::set_base_hook<boost::intrusive::optimize_size<true>> {
@@ -606,7 +595,6 @@ public:
 
     ExtentMap(Onode *o);
     ~ExtentMap() {
-      spanning_blob_map.clear_and_dispose([&](Blob *b) { b->put(); });
       extent_map.clear_and_dispose([&](Extent *e) { delete e; });
     }
 
@@ -617,7 +605,11 @@ public:
     void encode_spanning_blobs(bufferlist& bl);
     void decode_spanning_blobs(Collection *c, bufferlist::iterator& p);
 
-    BlobRef get_spanning_blob(int id);
+    BlobRef get_spanning_blob(int id) {
+      auto p = spanning_blob_map.find(id);
+      assert(p != spanning_blob_map.end());
+      return p->second;
+    }
 
     bool update(Onode *on, KeyValueDB::Transaction t, bool force);
     void reshard(Onode *on, uint64_t min_alloc_size);