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 <aemerson@redhat.com>

diff --git a/src/rgw/rgw_cache.cc b/src/rgw/rgw_cache.cc
index 67c8d834aba18ba48d619d8f043ac19db605d2be..9706a8e33a49192ee005677213a4dd516fd0be7d 100644 (file)
--- 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<rgw_cache_entry_info *>& cache_info_entries, RGWChainedCache::Entry *chained_entry)
+bool ObjectCache::chain_cache_entry(std::initializer_list<rgw_cache_entry_info*> cache_info_entries,
+                                   RGWChainedCache::Entry *chained_entry)
 {
   RWLock::WLocker l(lock);
 
@@ -76,40 +77,35 @@ bool ObjectCache::chain_cache_entry(list<rgw_cache_entry_info *>& cache_info_ent
     return false;
   }
 
-  list<rgw_cache_entry_info *>::iterator citer;
-
-  list<ObjectCacheEntry *> cache_entry_list;
-
+  std::vector<ObjectCacheEntry*> 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<string, ObjectCacheEntry>::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<ObjectCacheEntry *>::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<string, ObjectCacheEntry>::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<pair<RGWChainedCache *, string> >::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<string>::iterator& lru_iter)
+void ObjectCache::touch_lru(string& name, ObjectCacheEntry& entry,
+                           std::deque<string>::iterator& lru_iter)
 {
   while (lru_size > (size_t)cct->_conf->rgw_cache_lru_size) {
-    list<string>::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<str
        */
       break;
     }
-    map<string, ObjectCacheEntry>::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<str
   entry.lru_promotion_ts = lru_counter;
 }
 
-void ObjectCache::remove_lru(string& name, std::list<string>::iterator& lru_iter)
+void ObjectCache::remove_lru(string& name,
+                            std::deque<string>::iterator& lru_iter)
 {
   if (lru_iter == lru.end())
     return;
@@ -264,7 +258,7 @@ void ObjectCache::remove_lru(string& name, std::list<string>::iterator& lru_iter
 
 void ObjectCache::invalidate_lru(ObjectCacheEntry& entry)
 {
-  for (list<pair<RGWChainedCache *, string> >::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<RGWChainedCache *>::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 4c7c1adac37ff6d79bee091d1825752e492e9dce..608c8b27c287a79ec941927d59ac5c0254a0d73d 100644 (file)
--- 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<string>::iterator lru_iter;
+  std::deque<string>::iterator lru_iter;
   uint64_t lru_promotion_ts;
   uint64_t gen;
-  std::list<pair<RGWChainedCache *, string> > chained_entries;
+  std::vector<pair<RGWChainedCache *, string> > chained_entries;
 
   ObjectCacheEntry() : lru_promotion_ts(0), gen(0) {}
 };
 
 class ObjectCache {
   std::map<string, ObjectCacheEntry> cache_map;
-  std::list<string> lru;
+  std::deque<string> lru;
   unsigned long lru_size;
   unsigned long lru_counter;
   unsigned long lru_window;
   RWLock lock;
   CephContext *cct;
 
-  list<RGWChainedCache *> chained_cache;
+  vector<RGWChainedCache *> chained_cache;
 
   bool enabled;
 
-  void touch_lru(string& name, ObjectCacheEntry& entry, std::list<string>::iterator& lru_iter);
-  void remove_lru(string& name, std::list<string>::iterator& lru_iter);
+  void touch_lru(string& name, ObjectCacheEntry& entry,
+                std::deque<string>::iterator& lru_iter);
+  void remove_lru(string& name, std::deque<string>::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<rgw_cache_entry_info *>& cache_info_entries, RGWChainedCache::Entry *chained_entry);
+  bool chain_cache_entry(std::initializer_list<rgw_cache_entry_info*> 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<rgw_cache_entry_info *>& cache_info_entries, RGWChainedCache::Entry *chained_entry) override {
+  bool chain_cache_entry(std::initializer_list<rgw_cache_entry_info *> 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 2cc1d628d43744faaa4b126da3baa7aa602d37b2..4bca3138b42576a36695bd5b3d5c056432950c82 100644 (file)
--- 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<rgw_cache_entry_info *> 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 4240e5a53a4fadba9d5f11d1862f6d78197adc41..e45c81fec0d735c885ab276716258fbdd5755795 100644 (file)
--- 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<rgw_cache_entry_info *>& cache_info_entries, RGWChainedCache::Entry *chained_entry) { return false; }
+  virtual bool chain_cache_entry(std::initializer_list<rgw_cache_entry_info*> 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<rgw_cache_entry_info *>& cache_info_entries) {
+  bool put(RGWRados *store, const string& key, T *entry,
+          std::initializer_list<rgw_cache_entry_info *> 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 6a968efaa4be2ad4f5dc7c7177585f67bef5f491..f8712afcf89c3120ae30ebbccd078522696f5344 100644 (file)
--- 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<rgw_cache_entry_info *> 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)