From 049797641b946a1f8a8c4484143859b9ba356146 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 14 Oct 2016 17:50:33 -0400 Subject: [PATCH] mempool: dynamic index by type in pool_t This avoids having to use statics for the pool_allocators and guessing what intenral types the containers are going to need. It'll be a bit slower in debug on on construction and destruction, but who cares! Signed-off-by: Sage Weil --- src/global/mempool.cc | 14 +- src/include/mempool.h | 226 +++++++--------------------- src/os/bluestore/BlueStore.cc | 17 --- src/os/bluestore/bluestore_types.cc | 3 - src/test/test_mempool.cc | 14 -- 5 files changed, 60 insertions(+), 214 deletions(-) diff --git a/src/global/mempool.cc b/src/global/mempool.cc index 614c8ce7327..209cee0d3e7 100644 --- a/src/global/mempool.cc +++ b/src/global/mempool.cc @@ -72,6 +72,7 @@ size_t mempool::pool_t::allocated_bytes() const for (size_t i = 0; i < num_shards; ++i) { result += shard[i].bytes; } + assert(result >= 0); return (size_t) result; } @@ -81,6 +82,7 @@ size_t mempool::pool_t::allocated_items() const for (size_t i = 0; i < num_shards; ++i) { result += shard[i].items; } + assert(result >= 0); return (size_t) result; } @@ -94,15 +96,11 @@ void mempool::pool_t::get_stats( } if (debug) { std::unique_lock shard_lock(lock); - for (const list_member_t *p = types.next; - p != &types; - p = p->next) { - const pool_allocator_base_t *c = - reinterpret_cast(p); - std::string n = ceph_demangle(c->type_id); + for (auto &p : type_map) { + std::string n = ceph_demangle(p.second.type_name); stats_t &s = (*by_type)[n]; - s.bytes = c->items * c->item_size; - s.items = c->items; + s.bytes = p.second.items * p.second.item_size; + s.items = p.second.items; } } } diff --git a/src/include/mempool.h b/src/include/mempool.h index e1761160a6f..7a0b5439c9a 100644 --- a/src/include/mempool.h +++ b/src/include/mempool.h @@ -102,44 +102,11 @@ BlueStore::Onode, we need to do (This is just because we need to name some static varables and we can't use :: in a variable name.) -In order to use the STL containers, a few additional declarations -are needed. For example, +In order to use the STL containers, simply use the namespaced variant +of the container type. For example, unittest_1::map myvec; -requires - - MEMPOOL_DEFINE_FACTORY(int, int, unittest_1); - MEMPOOL_DEFINE_MAP_FACTORY(int, int, unittest_1); - -There are similar macros for SET, LIST, and UNORDERED_MAP. The MAP -macro serves both std::map and std::multimap, and std::vector doesn't -need a second container-specific declaration (because it only -allocates T and T* arrays; there is no internal container-specific -wrapper type). - -unordered_map is trickier. First, it has to allocate the hash -buckets, which requires an extra mempool-wide definition that is -shared by all different types. Second, sometimes the hash value is -cached in the hash node and sometimes it is not. The glibc STL makes -its own decision if you don't explicitly define traits, so you either -need to match your definition with its inference, or explicitly define -traits, or simply define the allocator for with the cached and -uncached case and leave one of them unused (but polluting your -debug dumps). For example, - - unittest_2::unordered_map one; // not cached - unittest_2::unordered_map one; // cached - -needs - - MEMPOOL_DEFINE_UNORDERED_MAP_BASE_FACTORY(unittest_2); // used by both - MEMPOOL_DEFINE_UNORDERED_MAP_FACTORY(uint64_t, std::string, false, int_str, - unittest_2); - MEMPOOL_DEFINE_UNORDERED_MAP_FACTORY(std::string, uint64_t, true, str_int, - unittest_2); - - Introspection ------------- @@ -190,27 +157,6 @@ extern void set_debug_mode(bool d); struct pool_allocator_base_t; class pool_t; -// doubly linked list -struct list_member_t { - list_member_t *next; - list_member_t *prev; - list_member_t() : next(this), prev(this) {} - ~list_member_t() { - assert(next == this && prev == this); - } - void insert(list_member_t *i) { - i->next = next; - i->prev = this; - next = i; - } - void remove() { - prev->next = next; - next->prev = prev; - next = this; - prev = this; - } -}; - // we shard pool stats across many shard_t's to reduce the amount // of cacheline ping pong. enum { @@ -234,34 +180,27 @@ struct stats_t { } }; -// Root of all allocators, this enables the container information to -// operation easily. These fields are "always" accurate. -struct pool_allocator_base_t { - list_member_t list_member; // this must come first; see get_stats() hackery - - pool_t *pool = nullptr; - const char *type_id = nullptr; - size_t item_size = 0; +pool_t& get_pool(pool_index_t ix); - // for debug mode +struct type_t { + const char *type_name; + size_t item_size; std::atomic items = {0}; // signed - - // effective constructor - void attach_pool(pool_index_t index, const char *type_id); - - ~pool_allocator_base_t(); }; -pool_t& get_pool(pool_index_t ix); +struct type_info_hash { + std::size_t operator()(const std::type_info& k) const { + return k.hash_code(); + } +}; class pool_t { std::string name; shard_t shard[num_shards]; mutable std::mutex lock; // only used for types list - list_member_t types; // protected by lock + std::unordered_map type_map; - friend class pool_allocator_base_t; public: bool debug; @@ -287,6 +226,18 @@ public: return &shard[i]; } + type_t *get_type(const std::type_info& ti, size_t size) { + std::lock_guard l(lock); + auto p = type_map.find(ti.name()); + if (p != type_map.end()) { + return &p->second; + } + type_t &t = type_map[ti.name()]; + t.type_name = ti.name(); + t.item_size = size; + return &t; + } + // get pool stats. by_type is not populated if !debug void get_stats(stats_t *total, std::map *by_type) const; @@ -297,35 +248,15 @@ public: // skip unittest_[12] by default void dump(ceph::Formatter *f, size_t skip=2); -inline void pool_allocator_base_t::attach_pool( - pool_index_t index, - const char *_type_id) -{ - assert(pool == nullptr); - pool = &get_pool(index); - type_id = _type_id; - - // unconditionally register type, even if debug is currently off - std::unique_lock lock(pool->lock); - pool->types.insert(&list_member); -} - -inline pool_allocator_base_t::~pool_allocator_base_t() -{ - if (pool) { - std::unique_lock lock(pool->lock); - list_member.remove(); - } -} - -// Stateless STL allocator for use with containers. All actual state +// STL allocator for use with containers. All actual state // is stored in the static pool_allocator_base_t, which saves us from // passing the allocator to container constructors. template class pool_allocator { - static pool_allocator_base_t base; + pool_t *pool; + type_t *type = nullptr; public: typedef pool_allocator allocator_type; @@ -341,44 +272,45 @@ public: typedef pool_allocator other; }; + void init() { + pool = &get_pool(pool_ix); + if (pool) { + type = pool->get_type(typeid(T), sizeof(T)); + } + } + pool_allocator() { - // initialize fields in the static member. this should only happen - // once, but it's also harmless if we do it multiple times. - base.type_id = typeid(T).name(); - base.item_size = sizeof(T); + init(); } template - pool_allocator(const pool_allocator&) {} - void operator=(const allocator_type&) {} - - void attach_pool(pool_index_t index, const char *type_id) { - base.attach_pool(index, type_id); + pool_allocator(const pool_allocator&) { + init(); } - pointer allocate(size_t n, void *p = nullptr) { + T* allocate(size_t n, void *p = nullptr) { size_t total = sizeof(T) * n; - shard_t *shard = base.pool->pick_a_shard(); + shard_t *shard = pool->pick_a_shard(); shard->bytes += total; shard->items += n; - if (base.pool->debug) { - base.items += n; + if (pool->debug) { + type->items += n; } - pointer r = reinterpret_cast(new char[total]); + T* r = reinterpret_cast(new char[total]); return r; } - void deallocate(pointer p, size_type n) { + void deallocate(T* p, size_t n) { size_t total = sizeof(T) * n; - shard_t *shard = base.pool->pick_a_shard(); + shard_t *shard = pool->pick_a_shard(); shard->bytes -= total; shard->items -= n; - if (base.pool->debug) { - base.items -= n; + if (type) { + type->items -= n; } delete[] reinterpret_cast(p); } - void destroy(pointer p) { + void destroy(T* p) { p->~T(); } @@ -387,7 +319,7 @@ public: p->~U(); } - void construct(pointer p, const_reference val) { + void construct(T* p, const T& val) { ::new ((void *)p) T(val); } @@ -395,8 +327,8 @@ public: ::new((void *)p) U(std::forward(args)...); } - bool operator==(const pool_allocator&) { return true; } - bool operator!=(const pool_allocator&) { return false; } + bool operator==(const pool_allocator&) const { return true; } + bool operator!=(const pool_allocator&) const { return false; } }; @@ -409,9 +341,6 @@ public: typedef pool_allocator allocator_type; static allocator_type alloc; - factory() { - alloc.attach_pool(pool_ix, typeid(o).name()); - } static void *allocate() { return (void *)alloc.allocate(1); } @@ -464,13 +393,7 @@ DEFINE_MEMORY_POOLS_HELPER(P) // Use this for any type that is contained by a container (unless it // is a class you defined; see below). #define MEMPOOL_DEFINE_FACTORY(obj, factoryname, pool) \ - template<> \ - mempool::pool_allocator_base_t \ - mempool::pool_allocator::base = {}; \ - template<> \ - typename pool::factory::allocator_type \ - pool::factory::alloc = {}; \ - static pool::factory _factory_##factoryname; + pool::pool_allocator _factory_##pool##factoryname##_alloc = {}; // Use this for each class that belongs to a mempool. For example, // @@ -489,52 +412,11 @@ DEFINE_MEMORY_POOLS_HELPER(P) // MEMPOOL_CLASS_HELPERS(). #define MEMPOOL_DEFINE_OBJECT_FACTORY(obj,factoryname,pool) \ MEMPOOL_DEFINE_FACTORY(obj, factoryname, pool) \ - void * obj::operator new(size_t size) { \ - assert(size == sizeof(obj)); \ - return pool::factory::allocate(); \ + void *obj::operator new(size_t size) { \ + return _factory_##pool##factoryname##_alloc.allocate(1); \ } \ void obj::operator delete(void *p) { \ - pool::factory::free(p); \ + return _factory_##pool##factoryname##_alloc.deallocate((obj*)p, 1); \ } -// for std::set -#define MEMPOOL_DEFINE_SET_FACTORY(t, factoryname, pool) \ - MEMPOOL_DEFINE_FACTORY(std::_Rb_tree_node, \ - factoryname##_rbtree_node, pool); - -// for std::list -#define MEMPOOL_DEFINE_LIST_FACTORY(t, factoryname, pool) \ - MEMPOOL_DEFINE_FACTORY(std::_List_node, \ - factoryname##_list_node, pool); - -// for std::map -#define MEMPOOL_DEFINE_MAP_FACTORY(k, v, factoryname, pool) \ - typedef std::pair _factory_type_##factoryname##pair_t; \ - MEMPOOL_DEFINE_FACTORY( \ - _factory_type_##factoryname##pair_t, \ - factoryname##_pair, pool); \ - typedef std::pair _factory_type_##factoryname##pair_t; \ - MEMPOOL_DEFINE_FACTORY( \ - std::_Rb_tree_node<_factory_type_##factoryname##pair_t>, \ - factoryname##_rbtree_node, pool); - -// for std::unordered_map -#define MEMPOOL_DEFINE_UNORDERED_MAP_FACTORY(k, v, cached, factoryname, pool) \ - typedef std::pair _factory_type_##factoryname##pair_t; \ - typedef std::pair _factory_type_##factoryname##cpair_t; \ - typedef std::__detail::_Hash_node<_factory_type_##factoryname##cpair_t, \ - cached> \ - _factory_type_##factoryname##type; \ - MEMPOOL_DEFINE_FACTORY( \ - _factory_type_##factoryname##type, \ - factoryname##_unordered_hash_node, pool); \ - MEMPOOL_DEFINE_FACTORY( \ - _factory_type_##factoryname##pair_t, \ - factoryname##_unordered_hash_pair, pool); - -#define MEMPOOL_DEFINE_UNORDERED_MAP_BASE_FACTORY(pool) \ - MEMPOOL_DEFINE_FACTORY(std::__detail::_Hash_node_base*, \ - pool##_unordered_hash_node_ptr, pool); - - #endif diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index c96ed46db3a..61b7228fba9 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -41,29 +41,12 @@ MEMPOOL_DEFINE_OBJECT_FACTORY(BlueStore::Onode, bluestore_onode, // bluestore_meta_other MEMPOOL_DEFINE_OBJECT_FACTORY(BlueStore::Buffer, bluestore_buffer, bluestore_meta_other); -MEMPOOL_DEFINE_MAP_FACTORY(uint64_t, std::unique_ptr, - bluestore_uint64_Buffer, bluestore_meta_other); MEMPOOL_DEFINE_OBJECT_FACTORY(BlueStore::Extent, bluestore_extent, bluestore_meta_other); MEMPOOL_DEFINE_OBJECT_FACTORY(BlueStore::Blob, bluestore_blob, bluestore_meta_other); -MEMPOOL_DEFINE_MAP_FACTORY(int, BlueStore::BlobRef, - bluestore_int_BlobRef, bluestore_meta_other); MEMPOOL_DEFINE_OBJECT_FACTORY(BlueStore::SharedBlob, bluestore_shared_blob, bluestore_meta_other); -MEMPOOL_DEFINE_FACTORY(BlueStore::ExtentMap::Shard, bluestore_ExtentMap_Shard, - bluestore_meta_other); - -MEMPOOL_DEFINE_UNORDERED_MAP_BASE_FACTORY(bluestore_meta_other); -MEMPOOL_DEFINE_UNORDERED_MAP_FACTORY(ghobject_t, BlueStore::OnodeRef, true, - bluestore_ghobject_OnodeRef, - bluestore_meta_other); -MEMPOOL_DEFINE_UNORDERED_MAP_FACTORY(coll_t, BlueStore::CollectionRef, true, - bluestore_coll_CollectionRef, - bluestore_meta_other); -MEMPOOL_DEFINE_UNORDERED_MAP_FACTORY(uint64_t, BlueStore::SharedBlob*, false, - bluestore_u64_sharedblob, - bluestore_meta_other); // kv store prefixes const string PREFIX_SUPER = "S"; // field -> value diff --git a/src/os/bluestore/bluestore_types.cc b/src/os/bluestore/bluestore_types.cc index bbb8287162f..028d7f60393 100644 --- a/src/os/bluestore/bluestore_types.cc +++ b/src/os/bluestore/bluestore_types.cc @@ -112,9 +112,6 @@ void bluestore_cnode_t::generate_test_instances(list& o) // bluestore_extent_ref_map_t -MEMPOOL_DEFINE_MAP_FACTORY(uint64_t, bluestore_extent_ref_map_t::record_t, - uint64_extent_ref_map_record, bluestore_meta_other) - void bluestore_extent_ref_map_t::_check() const { uint64_t pos = 0; diff --git a/src/test/test_mempool.cc b/src/test/test_mempool.cc index 9f23e3cd771..73c90b985d3 100644 --- a/src/test/test_mempool.cc +++ b/src/test/test_mempool.cc @@ -204,8 +204,6 @@ TEST(mempool, test_factory) check_usage(mempool::unittest_2); } -MEMPOOL_DEFINE_FACTORY(int, int, unittest_1); - TEST(mempool, vector) { { @@ -220,9 +218,6 @@ TEST(mempool, vector) } } -MEMPOOL_DEFINE_SET_FACTORY(int, int, unittest_1); -MEMPOOL_DEFINE_SET_FACTORY(obj, obj, unittest_2); - TEST(mempool, set) { unittest_1::set set_int; @@ -234,9 +229,6 @@ TEST(mempool, set) set_obj.insert(obj(1, 2)); } -MEMPOOL_DEFINE_MAP_FACTORY(int, int, int_int, unittest_1); -MEMPOOL_DEFINE_MAP_FACTORY(int, obj, int_obj, unittest_2); - TEST(mempool, map) { { @@ -252,9 +244,6 @@ TEST(mempool, map) } } -MEMPOOL_DEFINE_LIST_FACTORY(int, int, unittest_1); -MEMPOOL_DEFINE_LIST_FACTORY(obj, obj, unittest_2); - TEST(mempool, list) { { @@ -269,9 +258,6 @@ TEST(mempool, list) } } -MEMPOOL_DEFINE_UNORDERED_MAP_BASE_FACTORY(unittest_2); -MEMPOOL_DEFINE_UNORDERED_MAP_FACTORY(int, obj, false, int_obj, unittest_2); - TEST(mempool, unordered_map) { unittest_2::unordered_map h; -- 2.39.5