os/bluestore: use std::unordered_map for SharedBlob lookup

Many blobs aren't shared.  Save 8 bytes per SharedBlob by using a normal
unordered_map instead of instrusive::set.

More importantly, perhaps, it avoids us having to tune the intrusive
unordered_set size manually.  std::unordered_map does this automatically
for you, but the intrusive one does not.  And it's unclear how to
statically size it given that it's a per-collection structure and we have
no idea how many objects we'll have, how many blobs per object, and how
many objects will be cloned.

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

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 5d53f227c6f0a64ab96364ebf4ee53e92138c8d5..27b78173d96123716aaa23278dfe1976b112c850 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -2187,11 +2187,6 @@ BlueStore::Collection::Collection(BlueStore *ns, Cache *c, coll_t cid)
     cid(cid),
     lock("BlueStore::Collection::lock", true, false),
     exists(true),
-    // size the shared blob hash table as a ratio of the onode cache size.
-    shared_blob_set(MAX(16,
-                       g_conf->bluestore_onode_cache_size /
-                       store->cache_shards.size() *
-                       g_conf->bluestore_shared_blob_hash_table_size_ratio)),
     onode_map(c)
 {
 }

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 0bfc7d75a8f8a4e0215d7b7c0538835f7520365c..78c44de938999de9aca3e5b9a24ab30a9602c457 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -318,7 +318,7 @@ public:
   struct SharedBlobSet;
 
   /// in-memory shared blob state (incl cached buffers)
-  struct SharedBlob : public boost::intrusive::unordered_set_base_hook<> {
+  struct SharedBlob {
     std::atomic_int nref = {0}; ///< reference count
 
     // these are defined/set if the shared_blob is 'loaded'
@@ -357,37 +357,24 @@ public:
 
   /// a lookup table of SharedBlobs
   struct SharedBlobSet {
-    typedef boost::intrusive::unordered_set<SharedBlob>::bucket_type bucket_type;
-    typedef boost::intrusive::unordered_set<SharedBlob>::bucket_traits bucket_traits;
-
     std::mutex lock;   ///< protect lookup, insertion, removal
-    int num_buckets;
-    vector<bucket_type> buckets;
-    boost::intrusive::unordered_set<SharedBlob> uset;
-
-    SharedBlob dummy;  ///< for lookups
 
-    explicit SharedBlobSet(unsigned n)
-      : num_buckets(n),
-       buckets(n),
-       uset(bucket_traits(buckets.data(), num_buckets)),
-       dummy(0, string(), nullptr) {
-      assert(n > 0);
-    }
+    // we use a bare pointer because we don't want to affect the ref
+    // count
+    std::unordered_map<uint64_t,SharedBlob*> sb_map;
 
     SharedBlobRef lookup(uint64_t sbid) {
       std::lock_guard<std::mutex> l(lock);
-      dummy.sbid = sbid;
-      auto p = uset.find(dummy);
-      if (p == uset.end()) {
+      auto p = sb_map.find(sbid);
+      if (p == sb_map.end()) {
         return nullptr;
       }
-      return &*p;
+      return p->second;
     }
 
     void add(SharedBlob *sb) {
       std::lock_guard<std::mutex> l(lock);
-      uset.insert(*sb);
+      sb_map[sb->sbid] = sb;
       sb->parent_set = this;
     }
 
@@ -395,7 +382,7 @@ public:
       std::lock_guard<std::mutex> l(lock);
       if (sb->nref == 0) {
        assert(sb->parent_set == this);
-       uset.erase(*sb);
+       sb_map.erase(sb->sbid);
        return true;
       }
       return false;
@@ -403,7 +390,7 @@ public:
 
     bool empty() {
       std::lock_guard<std::mutex> l(lock);
-      return uset.empty();
+      return sb_map.empty();
     }
   };
 

diff --git a/src/test/objectstore/test_bluestore_types.cc b/src/test/objectstore/test_bluestore_types.cc
index 2c2911dd56a67590903eda03bad4153c8e48fd48..e2c4028cdef4d46d69701ab7f26933baccf5bcab 100644 (file)
--- a/src/test/objectstore/test_bluestore_types.cc
+++ b/src/test/objectstore/test_bluestore_types.cc
@@ -34,6 +34,7 @@ TEST(bluestore, sizeof) {
   P(std::atomic_int);
   P(BlueStore::SharedBlobRef);
   P(boost::intrusive::set_base_hook<>);
+  P(boost::intrusive::unordered_set_base_hook<>);
   P(bufferlist);
   cout << "map<uint64_t,uint64_t>\t" << sizeof(map<uint64_t,uint64_t>) << std::endl;
   cout << "map<char,char>\t" << sizeof(map<char,char>) << std::endl;