Properly test both INVALIDATE and UPDATE strategy. Fix bug with incorrect size calculations on updating existing elements.

3 files changed, 27 insertions, 41 deletions

diff --git a/staging_vespalib/src/tests/stllike/cache_test.cpp b/staging_vespalib/src/tests/stllike/cache_test.cpp
index 142c4065673..6e852644602 100644
--- a/staging_vespalib/src/tests/stllike/cache_test.cpp
+++ b/staging_vespalib/src/tests/stllike/cache_test.cpp
@@ -28,36 +28,10 @@ public:
     }
 };
 
-class Test : public TestApp
-{
-public:
-    int Main() override;
-private:
-    typedef LruParam<uint32_t, string> P;
-    typedef Map<uint32_t, string> B;
-    void testCache();
-    void testCacheSize();
-    void testCacheSizeDeep();
-    void testCacheEntriesHonoured();
-    void testCacheMaxSizeHonoured();
-    void testThatMultipleRemoveOnOverflowIsFine();
-};
-
-int
-Test::Main()
-{
-    TEST_INIT("cache_test");
-    testCache();
-    testCacheSize();
-    testCacheSizeDeep();
-    testCacheEntriesHonoured();
-    testCacheMaxSizeHonoured();
-    testThatMultipleRemoveOnOverflowIsFine();
-    TEST_DONE();
-}
+using P = LruParam<uint32_t, string>;
+using B = Map<uint32_t, string>;
 
-void Test::testCache()
-{
+TEST("testCache") {
     B m;
     cache< CacheParam<P, B> > cache(m, -1);
     // Verfify start conditions.
@@ -74,24 +48,29 @@ void Test::testCache()
     EXPECT_TRUE(cache.size() == 1);
 }
 
-void Test::testCacheSize()
+TEST("testCacheSize")
 {
     B m;
     cache< CacheParam<P, B> > cache(m, -1);
     cache.write(1, "10 bytes string");
     EXPECT_EQUAL(80u, cache.sizeBytes());
+    cache.write(1, "10 bytes string"); // Still the same size
+    EXPECT_EQUAL(80u, cache.sizeBytes());
 }
 
-void Test::testCacheSizeDeep()
+TEST("testCacheSizeDeep")
 {
     B m;
     cache< CacheParam<P, B, zero<uint32_t>, size<string> > > cache(m, -1);
     cache.write(1, "15 bytes string");
     EXPECT_EQUAL(95u, cache.sizeBytes());
+    cache.write(1, "10 bytes s");
+    EXPECT_EQUAL(90u, cache.sizeBytes());
+    cache.write(1, "20 bytes string ssss");
+    EXPECT_EQUAL(100u, cache.sizeBytes());
 }
 
-void Test::testCacheEntriesHonoured()
-{
+TEST("testCacheEntriesHonoured") {
     B m;
     cache< CacheParam<P, B, zero<uint32_t>, size<string> > > cache(m, -1);
     cache.maxElements(1);
@@ -105,8 +84,7 @@ void Test::testCacheEntriesHonoured()
     EXPECT_EQUAL(96u, cache.sizeBytes());
 }
 
-void Test::testCacheMaxSizeHonoured()
-{
+TEST("testCacheMaxSizeHonoured") {
     B m;
     cache< CacheParam<P, B, zero<uint32_t>, size<string> > > cache(m, 200);
     cache.write(1, "15 bytes string");
@@ -123,8 +101,7 @@ void Test::testCacheMaxSizeHonoured()
     EXPECT_EQUAL(291u, cache.sizeBytes());
 }
 
-void Test::testThatMultipleRemoveOnOverflowIsFine()
-{
+TEST("testThatMultipleRemoveOnOverflowIsFine") {
     B m;
     cache< CacheParam<P, B, zero<uint32_t>, size<string> > > cache(m, 2000);
     
@@ -159,5 +136,4 @@ void Test::testThatMultipleRemoveOnOverflowIsFine()
     EXPECT_EQUAL(2924u, cache.sizeBytes());
 }
 
-
-TEST_APPHOOK(Test)
+TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/staging_vespalib/src/vespa/vespalib/stllike/cache.h b/staging_vespalib/src/vespa/vespalib/stllike/cache.h
index 6b3cec659aa..99b601f7c3c 100644
--- a/staging_vespalib/src/vespa/vespalib/stllike/cache.h
+++ b/staging_vespalib/src/vespa/vespalib/stllike/cache.h
@@ -148,6 +148,7 @@ private:
     mutable size_t      _race;
     mutable size_t      _insert;
     mutable size_t      _write;
+    mutable size_t      _update;
     mutable size_t      _erase;
     mutable size_t      _invalidate;
     mutable size_t      _lookup;
diff --git a/staging_vespalib/src/vespa/vespalib/stllike/cache.hpp b/staging_vespalib/src/vespa/vespalib/stllike/cache.hpp
index 1ba168fa77b..906621d623c 100644
--- a/staging_vespalib/src/vespa/vespalib/stllike/cache.hpp
+++ b/staging_vespalib/src/vespa/vespalib/stllike/cache.hpp
@@ -61,6 +61,7 @@ cache<P>::cache(BackingStore & b, size_t maxBytes) :
     _race(0),
     _insert(0),
     _write(0),
+    _update(0),
     _erase(0),
     _invalidate(0),
     _lookup(0),
@@ -122,13 +123,21 @@ template< typename P >
 void
 cache<P>::write(const K & key, V value)
 {
+    size_t newSize = calcSize(key, value);
     vespalib::LockGuard storeGuard(getLock(key));
+    {
+        vespalib::LockGuard guard(_hashLock);
+        if (Lru::hasKey(key)) {
+            _sizeBytes -= calcSize(key, (*this)[key]);
+            _update++;
+        }
+    }
+
     _store.write(key, value);
     {
         vespalib::LockGuard guard(_hashLock);
-        size_t sz = calcSize(key, value);
         (*this)[key] = std::move(value);
-        _sizeBytes += sz;
+        _sizeBytes += newSize;
         _write++;
     }
 }