From: Igor Fedotov Date: Fri, 13 Jan 2017 13:39:20 +0000 (+0000) Subject: os/bluestore: optimize SharedBlob::sbid in-mem representation. X-Git-Tag: v12.0.0~30^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a7f8e231e35c7624421b494a30623becfd1b4061;p=ceph.git os/bluestore: optimize SharedBlob::sbid in-mem representation. Signed-off-by: Igor Fedotov --- diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 6b6263afe10..54db484ac34 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -1367,19 +1367,19 @@ bool BlueStore::OnodeSpace::map_any(std::function 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 aff2f541318..d6c57869a67 100644 --- 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 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 l(lock); - sb_map[sb->sbid] = sb; + sb_map[sb->get_sbid()] = sb; sb->coll = coll; } @@ -402,7 +408,7 @@ public: std::lock_guard 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 4da726faa1b..6103612a72d 100644 --- 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& 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 610f1fe29b2..7b14b87e4cb 100644 --- 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& ls);