From: Adam C. Emerson Date: Mon, 18 Dec 2017 19:13:08 +0000 (-0500) Subject: rgw: Cache data structure cleanup X-Git-Tag: v13.0.2~675^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=afa51dea69e4e76e5c456849812a3956f3a1015d;p=ceph.git rgw: Cache data structure cleanup Do not use std::list for the LRU. And really don't cons up a std::list just to pass a variable number of arguments to a function. (Use initializer_list instead.) Signed-off-by: Adam C. Emerson --- diff --git a/src/rgw/rgw_cache.cc b/src/rgw/rgw_cache.cc index 67c8d834aba1..9706a8e33a49 100644 --- a/src/rgw/rgw_cache.cc +++ b/src/rgw/rgw_cache.cc @@ -68,7 +68,8 @@ int ObjectCache::get(string& name, ObjectCacheInfo& info, uint32_t mask, rgw_cac return 0; } -bool ObjectCache::chain_cache_entry(list& cache_info_entries, RGWChainedCache::Entry *chained_entry) +bool ObjectCache::chain_cache_entry(std::initializer_list cache_info_entries, + RGWChainedCache::Entry *chained_entry) { RWLock::WLocker l(lock); @@ -76,40 +77,35 @@ bool ObjectCache::chain_cache_entry(list& cache_info_ent return false; } - list::iterator citer; - - list cache_entry_list; - + std::vector entries; + entries.reserve(cache_info_entries.size()); /* first verify that all entries are still valid */ - for (citer = cache_info_entries.begin(); citer != cache_info_entries.end(); ++citer) { - rgw_cache_entry_info *cache_info = *citer; - - ldout(cct, 10) << "chain_cache_entry: cache_locator=" << cache_info->cache_locator << dendl; - map::iterator iter = cache_map.find(cache_info->cache_locator); + for (auto cache_info : cache_info_entries) { + ldout(cct, 10) << "chain_cache_entry: cache_locator=" + << cache_info->cache_locator << dendl; + auto iter = cache_map.find(cache_info->cache_locator); if (iter == cache_map.end()) { ldout(cct, 20) << "chain_cache_entry: couldn't find cache locator" << dendl; return false; } - ObjectCacheEntry *entry = &iter->second; + auto entry = &iter->second; if (entry->gen != cache_info->gen) { - ldout(cct, 20) << "chain_cache_entry: entry.gen (" << entry->gen << ") != cache_info.gen (" << cache_info->gen << ")" << dendl; + ldout(cct, 20) << "chain_cache_entry: entry.gen (" << entry->gen + << ") != cache_info.gen (" << cache_info->gen << ")" + << dendl; return false; } - - cache_entry_list.push_back(entry); + entries.push_back(entry); } chained_entry->cache->chain_cb(chained_entry->key, chained_entry->data); - list::iterator liter; - - for (liter = cache_entry_list.begin(); liter != cache_entry_list.end(); ++liter) { - ObjectCacheEntry *entry = *liter; - - entry->chained_entries.push_back(make_pair(chained_entry->cache, chained_entry->key)); + for (auto entry : entries) { + entry->chained_entries.push_back(make_pair(chained_entry->cache, + chained_entry->key)); } return true; @@ -125,7 +121,7 @@ void ObjectCache::put(string& name, ObjectCacheInfo& info, rgw_cache_entry_info ldout(cct, 10) << "cache put: name=" << name << " info.flags=0x" << std::hex << info.flags << std::dec << dendl; - map::iterator iter = cache_map.find(name); + auto iter = cache_map.find(name); if (iter == cache_map.end()) { ObjectCacheEntry entry; entry.lru_iter = lru.end(); @@ -135,8 +131,6 @@ void ObjectCache::put(string& name, ObjectCacheInfo& info, rgw_cache_entry_info ObjectCacheEntry& entry = iter->second; ObjectCacheInfo& target = entry.info; - invalidate_lru(entry); - entry.chained_entries.clear(); entry.gen++; @@ -203,20 +197,19 @@ void ObjectCache::remove(string& name) ldout(cct, 10) << "removing " << name << " from cache" << dendl; ObjectCacheEntry& entry = iter->second; - for (list >::iterator iiter = entry.chained_entries.begin(); - iiter != entry.chained_entries.end(); ++iiter) { - RGWChainedCache *chained_cache = iiter->first; - chained_cache->invalidate(iiter->second); + for (auto& kv : entry.chained_entries) { + kv.first->invalidate(kv.second); } remove_lru(name, iter->second.lru_iter); cache_map.erase(iter); } -void ObjectCache::touch_lru(string& name, ObjectCacheEntry& entry, std::list::iterator& lru_iter) +void ObjectCache::touch_lru(string& name, ObjectCacheEntry& entry, + std::deque::iterator& lru_iter) { while (lru_size > (size_t)cct->_conf->rgw_cache_lru_size) { - list::iterator iter = lru.begin(); + auto iter = lru.begin(); if ((*iter).compare(name) == 0) { /* * if the entry we're touching happens to be at the lru end, don't remove it, @@ -224,7 +217,7 @@ void ObjectCache::touch_lru(string& name, ObjectCacheEntry& entry, std::list::iterator map_iter = cache_map.find(*iter); + auto map_iter = cache_map.find(*iter); ldout(cct, 10) << "removing entry: name=" << *iter << " from cache LRU" << dendl; if (map_iter != cache_map.end()) { ObjectCacheEntry& entry = map_iter->second; @@ -252,7 +245,8 @@ void ObjectCache::touch_lru(string& name, ObjectCacheEntry& entry, std::list::iterator& lru_iter) +void ObjectCache::remove_lru(string& name, + std::deque::iterator& lru_iter) { if (lru_iter == lru.end()) return; @@ -264,7 +258,7 @@ void ObjectCache::remove_lru(string& name, std::list::iterator& lru_iter void ObjectCache::invalidate_lru(ObjectCacheEntry& entry) { - for (list >::iterator iter = entry.chained_entries.begin(); + for (auto iter = entry.chained_entries.begin(); iter != entry.chained_entries.end(); ++iter) { RGWChainedCache *chained_cache = iter->first; chained_cache->invalidate(iter->second); @@ -298,8 +292,8 @@ void ObjectCache::do_invalidate_all() lru_counter = 0; lru_window = 0; - for (list::iterator iter = chained_cache.begin(); iter != chained_cache.end(); ++iter) { - (*iter)->invalidate_all(); + for (auto& cache : chained_cache) { + cache->invalidate_all(); } } diff --git a/src/rgw/rgw_cache.h b/src/rgw/rgw_cache.h index 4c7c1adac37f..608c8b27c287 100644 --- a/src/rgw/rgw_cache.h +++ b/src/rgw/rgw_cache.h @@ -126,29 +126,30 @@ WRITE_CLASS_ENCODER(RGWCacheNotifyInfo) struct ObjectCacheEntry { ObjectCacheInfo info; - std::list::iterator lru_iter; + std::deque::iterator lru_iter; uint64_t lru_promotion_ts; uint64_t gen; - std::list > chained_entries; + std::vector > chained_entries; ObjectCacheEntry() : lru_promotion_ts(0), gen(0) {} }; class ObjectCache { std::map cache_map; - std::list lru; + std::deque lru; unsigned long lru_size; unsigned long lru_counter; unsigned long lru_window; RWLock lock; CephContext *cct; - list chained_cache; + vector chained_cache; bool enabled; - void touch_lru(string& name, ObjectCacheEntry& entry, std::list::iterator& lru_iter); - void remove_lru(string& name, std::list::iterator& lru_iter); + void touch_lru(string& name, ObjectCacheEntry& entry, + std::deque::iterator& lru_iter); + void remove_lru(string& name, std::deque::iterator& lru_iter); void invalidate_lru(ObjectCacheEntry& entry); void do_invalidate_all(); @@ -161,7 +162,8 @@ public: cct = _cct; lru_window = cct->_conf->rgw_cache_lru_size / 2; } - bool chain_cache_entry(list& cache_info_entries, RGWChainedCache::Entry *chained_entry); + bool chain_cache_entry(std::initializer_list cache_info_entries, + RGWChainedCache::Entry *chained_entry); void set_enabled(bool status); @@ -243,7 +245,7 @@ public: int delete_system_obj(rgw_raw_obj& obj, RGWObjVersionTracker *objv_tracker) override; - bool chain_cache_entry(list& cache_info_entries, RGWChainedCache::Entry *chained_entry) override { + bool chain_cache_entry(std::initializer_list cache_info_entries, RGWChainedCache::Entry *chained_entry) override { return cache.chain_cache_entry(cache_info_entries, chained_entry); } }; diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc index 2cc1d628d437..4bca3138b425 100644 --- a/src/rgw/rgw_rados.cc +++ b/src/rgw/rgw_rados.cc @@ -11983,13 +11983,8 @@ int RGWRados::_get_bucket_info(RGWObjectCtx& obj_ctx, if (pattrs) *pattrs = e.attrs; - list cache_info_entries; - cache_info_entries.push_back(&entry_cache_info); - cache_info_entries.push_back(&cache_info); - - /* chain to both bucket entry point and bucket instance */ - if (!binfo_cache->put(this, bucket_entry, &e, cache_info_entries)) { + if (!binfo_cache->put(this, bucket_entry, &e, {&entry_cache_info, &cache_info})) { ldout(cct, 20) << "couldn't put binfo cache entry, might have raced with data changes" << dendl; } diff --git a/src/rgw/rgw_rados.h b/src/rgw/rgw_rados.h index 4240e5a53a4f..e45c81fec0d7 100644 --- a/src/rgw/rgw_rados.h +++ b/src/rgw/rgw_rados.h @@ -3294,7 +3294,8 @@ public: rgw_cache_entry_info *cache_info); virtual void register_chained_cache(RGWChainedCache *cache) {} - virtual bool chain_cache_entry(list& cache_info_entries, RGWChainedCache::Entry *chained_entry) { return false; } + virtual bool chain_cache_entry(std::initializer_list cache_info_entries, + RGWChainedCache::Entry *chained_entry) { return false; } int iterate_obj(RGWObjectCtx& ctx, const RGWBucketInfo& bucket_info, const rgw_obj& obj, @@ -3784,7 +3785,8 @@ public: return iter->second.first; } - bool put(RGWRados *store, const string& key, T *entry, list& cache_info_entries) { + bool put(RGWRados *store, const string& key, T *entry, + std::initializer_list cache_info_entries) { Entry chain_entry(this, key, entry); /* we need the store cache to call us under its lock to maintain lock ordering */ diff --git a/src/rgw/rgw_user.cc b/src/rgw/rgw_user.cc index 6a968efaa4be..f8712afcf89c 100644 --- a/src/rgw/rgw_user.cc +++ b/src/rgw/rgw_user.cc @@ -294,10 +294,7 @@ int rgw_get_user_info_from_index(RGWRados * const store, return -EIO; } - list cache_info_entries; - cache_info_entries.push_back(&cache_info); - - uinfo_cache.put(store, key, &e, cache_info_entries); + uinfo_cache.put(store, key, &e, { &cache_info }); info = e.info; if (objv_tracker)