shared_cache: add purge and tests

purge detaches the lru shared_ptr currently associated from
the key from the lru even if there are still references.

Signed-off-by: Samuel Just <sam.just@inktank.com>

diff --git a/src/common/shared_cache.hpp b/src/common/shared_cache.hpp
index 0c4832c51647ccf7ec2ecde6b9788fca960ae9cf..cb478a52c2f90309b398f54a375848c839fb1740 100644 (file)
--- a/src/common/shared_cache.hpp
+++ b/src/common/shared_cache.hpp
@@ -35,7 +35,7 @@ class SharedLRU {
   map<K, typename list<pair<K, VPtr> >::iterator > contents;
   list<pair<K, VPtr> > lru;
 
-  map<K, WeakVPtr> weak_refs;
+  map<K, pair<WeakVPtr, V*> > weak_refs;
 
   void trim_cache(list<VPtr> *to_release) {
     while (size > max_size) {
@@ -67,9 +67,12 @@ class SharedLRU {
     }
   }
 
-  void remove(const K& key) {
+  void remove(const K& key, V *valptr) {
     Mutex::Locker l(lock);
-    weak_refs.erase(key);
+    typename map<K, pair<WeakVPtr, V*> >::iterator i = weak_refs.find(key);
+    if (i != weak_refs.end() && i->second.second == valptr) {
+      weak_refs.erase(i);
+    }
     cond.Signal();
   }
 
@@ -79,7 +82,7 @@ class SharedLRU {
     K key;
     Cleanup(SharedLRU<K, V> *cache, K key) : cache(cache), key(key) {}
     void operator()(V *ptr) {
-      cache->remove(key);
+      cache->remove(key, ptr);
       delete ptr;
     }
   };
@@ -110,12 +113,12 @@ public:
   }
 
   void dump_weak_refs(ostream& out) {
-    for (typename map<K, WeakVPtr>::iterator p = weak_refs.begin();
+    for (typename map<K, pair<WeakVPtr, V*> >::iterator p = weak_refs.begin();
         p != weak_refs.end();
         ++p) {
       out << __func__ << " " << this << " weak_refs: "
-         << p->first << " = " << p->second.lock().get()
-         << " with " << p->second.use_count() << " refs"
+         << p->first << " = " << p->second.second
+         << " with " << p->second.first.use_count() << " refs"
          << std::endl;
     }
   }
@@ -125,9 +128,21 @@ public:
     {
       Mutex::Locker l(lock);
       if (weak_refs.count(key)) {
-       val = weak_refs[key].lock();
+       val = weak_refs[key].first.lock();
+      }
+      lru_remove(key);
+    }
+  }
+
+  void purge(const K &key) {
+    VPtr val; // release any ref we have after we drop the lock
+    {
+      Mutex::Locker l(lock);
+      if (weak_refs.count(key)) {
+       val = weak_refs[key].first.lock();
       }
       lru_remove(key);
+      weak_refs.erase(key);
     }
   }
 
@@ -156,10 +171,11 @@ public:
        retry = false;
        if (weak_refs.empty())
          break;
-       typename map<K, WeakVPtr>::iterator i = weak_refs.lower_bound(key);
+       typename map<K, pair<WeakVPtr, V*> >::iterator i =
+         weak_refs.lower_bound(key);
        if (i == weak_refs.end())
          --i;
-       val = i->second.lock();
+       val = i->second.first.lock();
        if (val) {
          lru_add(i->first, val, &to_release);
        } else {
@@ -180,9 +196,9 @@ public:
       bool retry = false;
       do {
        retry = false;
-       typename map<K, WeakVPtr>::iterator i = weak_refs.find(key);
+       typename map<K, pair<WeakVPtr, V*> >::iterator i = weak_refs.find(key);
        if (i != weak_refs.end()) {
-         val = i->second.lock();
+         val = i->second.first.lock();
          if (val) {
            lru_add(key, val, &to_release);
          } else {
@@ -213,19 +229,20 @@ public:
     list<VPtr> to_release;
     {
       Mutex::Locker l(lock);
-      typename map<K, WeakVPtr>::iterator actual = weak_refs.lower_bound(key);
+      typename map<K, pair<WeakVPtr, V*> >::iterator actual =
+       weak_refs.lower_bound(key);
       if (actual != weak_refs.end() && actual->first == key) {
         if (existed) 
           *existed = true;
 
-        return actual->second.lock();
+        return actual->second.first.lock();
       }
 
       if (existed)      
         *existed = false;
 
       val = VPtr(value, Cleanup(this, key));
-      weak_refs.insert(actual, make_pair(key, val));
+      weak_refs.insert(actual, make_pair(key, make_pair(val, value)));
       lru_add(key, val, &to_release);
     }
     return val;

diff --git a/src/test/Makefile.am b/src/test/Makefile.am
index 2c9b8488563676399f982792ac81b0030581d436..bbc9c979ba7639dc74ae9749b2d516724e969c42 100644 (file)
--- a/src/test/Makefile.am
+++ b/src/test/Makefile.am
@@ -293,6 +293,11 @@ unittest_sharedptr_registry_CXXFLAGS = $(UNITTEST_CXXFLAGS)
 unittest_sharedptr_registry_LDADD = $(UNITTEST_LDADD) $(CEPH_GLOBAL)
 check_PROGRAMS += unittest_sharedptr_registry
 
+unittest_shared_cache_SOURCES = test/common/test_shared_cache.cc
+unittest_shared_cache_CXXFLAGS = $(UNITTEST_CXXFLAGS)
+unittest_shared_cache_LDADD = $(UNITTEST_LDADD) $(CEPH_GLOBAL)
+check_PROGRAMS += unittest_shared_cache
+
 unittest_sloppy_crc_map_SOURCES = test/common/test_sloppy_crc_map.cc
 unittest_sloppy_crc_map_CXXFLAGS = $(UNITTEST_CXXFLAGS)
 unittest_sloppy_crc_map_LDADD = $(UNITTEST_LDADD) $(CEPH_GLOBAL)

diff --git a/src/test/common/test_shared_cache.cc b/src/test/common/test_shared_cache.cc
new file mode 100644 (file)
index 0000000..bafef26
--- /dev/null
+++ b/src/test/common/test_shared_cache.cc
@@ -0,0 +1,77 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+/*
+ * Ceph - scalable distributed file system
+ *
+ * Copyright (C) 2013 Cloudwatt <libre.licensing@cloudwatt.com>
+ *
+ * Author: Loic Dachary <loic@dachary.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Library Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Library Public License for more details.
+ *
+ */
+
+#include <stdio.h>
+#include <signal.h>
+#include "common/Thread.h"
+#include "common/shared_cache.hpp"
+#include "common/ceph_argparse.h"
+#include "global/global_init.h"
+#include <gtest/gtest.h>
+
+using namespace std::tr1;
+
+TEST(SharedCache_all, add) {
+  SharedLRU<int, int> cache;
+  unsigned int key = 1;
+  int value = 2;
+  shared_ptr<int> ptr = cache.add(key, new int(value));
+  ASSERT_EQ(ptr, cache.lookup(key));
+  ASSERT_EQ(value, *cache.lookup(key));
+}
+
+TEST(SharedCache_all, lru) {
+  const size_t SIZE = 5;
+  SharedLRU<int, int> cache(NULL, SIZE);
+
+  bool existed = false;
+  shared_ptr<int> ptr = cache.add(0, new int(0), &existed);
+  ASSERT_FALSE(existed);
+  {
+    int *tmpint = new int(0);
+    shared_ptr<int> ptr2 = cache.add(0, tmpint, &existed);
+    ASSERT_TRUE(existed);
+    delete tmpint;
+  }
+  for (size_t i = 1; i < 2*SIZE; ++i) {
+    cache.add(i, new int(i), &existed);
+    ASSERT_FALSE(existed);
+  }
+
+  ASSERT_TRUE(cache.lookup(0));
+  ASSERT_EQ(0, *cache.lookup(0));
+
+  ASSERT_FALSE(cache.lookup(SIZE-1));
+  ASSERT_FALSE(cache.lookup(SIZE));
+  ASSERT_TRUE(cache.lookup(SIZE+1));
+  ASSERT_EQ(SIZE+1, *cache.lookup(SIZE+1));
+
+  cache.purge(0);
+  ASSERT_FALSE(cache.lookup(0));
+  shared_ptr<int> ptr2 = cache.add(0, new int(0), &existed);
+  ASSERT_FALSE(ptr == ptr2);
+  ptr = shared_ptr<int>();
+  ASSERT_TRUE(cache.lookup(0));
+}
+
+// Local Variables:
+// compile-command: "cd ../.. ; make unittest_sharedptr_registry && ./unittest_sharedptr_registry # --gtest_filter=*.* --log-to-stderr=true"
+// End: