Skip to content

Commit

Permalink
Reduce number of lookups in LRU map for updates and invalidations
Browse files Browse the repository at this point in the history 
 * Add `find_no_ref` method to `lrucache_map` which returns an iterator
   to the mapped element iff it exists, and which does _not_ update
   the LRU ordering.
 * Collapse 3 LRU map lookups into 1 for cache updates and invalidations
   for existing mappings.
 * Rewrite `lrucache_map` tests to use GTest
  • Loading branch information
@vekterli
vekterli committed Nov 27, 2024
1 parent 7ae77c0 commit 239bc8d
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 117 deletions.
1 change: 1 addition & 0 deletions vespalib/src/tests/stllike/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ vespa_add_executable(vespalib_lrucache_test_app TEST
lrucache.cpp
DEPENDS
vespalib
GTest::GTest
)
vespa_add_test(NAME vespalib_lrucache_test_app COMMAND vespalib_lrucache_test_app)
vespa_add_executable(vespalib_cache_test_app TEST
Expand Down
196 changes: 114 additions & 82 deletions vespalib/src/tests/stllike/lrucache.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,48 +1,48 @@
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.

#include <vespa/vespalib/testkit/test_kit.h>
#include <vespa/vespalib/gtest/gtest.h>
#include <vespa/vespalib/stllike/lrucache_map.hpp>
#include <string>

using namespace vespalib;

TEST("testCache") {
lrucache_map< LruParam<int, std::string> > cache(7);
// Verfify start conditions.
EXPECT_TRUE(cache.size() == 0);
TEST(LruCacheMapTest, cache_basics) {
lrucache_map<LruParam<int, std::string>> cache(7);
// Verify start conditions.
EXPECT_EQ(cache.size(), 0);
cache.insert(1, "First inserted string");
EXPECT_TRUE(cache.verifyInternals());
EXPECT_TRUE(cache.size() == 1);
EXPECT_EQ(cache.size(), 1);
EXPECT_TRUE(cache.hasKey(1));
cache.insert(2, "Second inserted string");
EXPECT_TRUE(cache.verifyInternals());
EXPECT_TRUE(cache.size() == 2);
EXPECT_EQ(cache.size(), 2);
EXPECT_TRUE(cache.hasKey(1));
EXPECT_TRUE(cache.hasKey(2));
cache.insert(3, "Third inserted string");
EXPECT_TRUE(cache.verifyInternals());
EXPECT_TRUE(cache.size() == 3);
EXPECT_EQ(cache.size(), 3);
EXPECT_TRUE(cache.hasKey(1));
EXPECT_TRUE(cache.hasKey(2));
EXPECT_TRUE(cache.hasKey(3));
cache.insert(4, "Fourth inserted string");
EXPECT_TRUE(cache.verifyInternals());
EXPECT_TRUE(cache.size() == 4);
EXPECT_EQ(cache.size(), 4);
EXPECT_TRUE(cache.hasKey(1));
EXPECT_TRUE(cache.hasKey(2));
EXPECT_TRUE(cache.hasKey(3));
EXPECT_TRUE(cache.hasKey(4));
cache.insert(5, "Fifth inserted string");
EXPECT_TRUE(cache.verifyInternals());
EXPECT_TRUE(cache.size() == 5);
EXPECT_EQ(cache.size(), 5);
EXPECT_TRUE(cache.hasKey(1));
EXPECT_TRUE(cache.hasKey(2));
EXPECT_TRUE(cache.hasKey(3));
EXPECT_TRUE(cache.hasKey(4));
EXPECT_TRUE(cache.hasKey(5));
cache.insert(6, "Sixt inserted string");
EXPECT_TRUE(cache.verifyInternals());
EXPECT_TRUE(cache.size() == 6);
EXPECT_EQ(cache.size(), 6);
EXPECT_TRUE(cache.hasKey(1));
EXPECT_TRUE(cache.hasKey(2));
EXPECT_TRUE(cache.hasKey(3));
Expand All @@ -51,7 +51,7 @@ TEST("testCache") {
EXPECT_TRUE(cache.hasKey(6));
cache.insert(7, "Seventh inserted string");
EXPECT_TRUE(cache.verifyInternals());
EXPECT_EQUAL(cache.size(), 7u);
EXPECT_EQ(cache.size(), 7);
EXPECT_TRUE(cache.hasKey(1));
EXPECT_TRUE(cache.hasKey(2));
EXPECT_TRUE(cache.hasKey(3));
Expand All @@ -61,7 +61,7 @@ TEST("testCache") {
EXPECT_TRUE(cache.hasKey(7));
cache.insert(8, "Eighth inserted string");
EXPECT_TRUE(cache.verifyInternals());
EXPECT_EQUAL(cache.size(), 7u);
EXPECT_EQ(cache.size(), 7);
EXPECT_TRUE(cache.hasKey(2));
EXPECT_TRUE(cache.hasKey(3));
EXPECT_TRUE(cache.hasKey(4));
Expand All @@ -71,7 +71,7 @@ TEST("testCache") {
EXPECT_TRUE(cache.hasKey(8));
cache.insert(15, "Eighth inserted string");
EXPECT_TRUE(cache.verifyInternals());
EXPECT_EQUAL(cache.size(), 7u);
EXPECT_EQ(cache.size(), 7);
EXPECT_TRUE(cache.hasKey(3));
EXPECT_TRUE(cache.hasKey(4));
EXPECT_TRUE(cache.hasKey(5));
Expand All @@ -80,7 +80,7 @@ TEST("testCache") {
EXPECT_TRUE(cache.hasKey(8));
EXPECT_TRUE(cache.hasKey(15));
// Test get and erase
cache.get(3);
(void)cache.get(3);
EXPECT_TRUE(cache.verifyInternals());
cache.erase(3);
EXPECT_TRUE(cache.verifyInternals());
Expand All @@ -91,135 +91,165 @@ using MyKey = std::shared_ptr<std::string>;
using MyData = std::shared_ptr<std::string>;

struct SharedEqual {
bool operator()(const MyKey & a, const MyKey & b) {
bool operator()(const MyKey& a, const MyKey& b) const noexcept {
return ((*a) == (*b));
}
};

struct SharedHash {
size_t operator() (const MyKey & arg) const { return arg->size(); }
size_t operator()(const MyKey& arg) const noexcept { return arg->size(); }
};


TEST("testCacheInsertOverResize") {
TEST(LruCacheMapTest, cache_insert_over_resize) {
using LS = std::shared_ptr<std::string>;
using Cache = lrucache_map< LruParam<int, LS> >;
using Cache = lrucache_map<LruParam<int, LS>>;

Cache cache(100);
size_t sum(0);
for (size_t i(0); i < cache.capacity()*10; i++) {
LS s(new std::string("abc"));
LS s(std::make_shared<std::string>("abc"));
cache[random()] = s;
sum += strlen(s->c_str());
EXPECT_EQUAL(strlen(s->c_str()), s->size());
EXPECT_EQ(strlen(s->c_str()), s->size());
}
EXPECT_EQUAL(sum, cache.capacity()*10*3);
EXPECT_EQ(sum, cache.capacity()*10*3);
}

TEST("testCacheErase") {
lrucache_map< LruParam<MyKey, MyData, SharedHash, SharedEqual> > cache(4);
TEST(LruCacheMapTest, cache_erase_by_key) {
lrucache_map<LruParam<MyKey, MyData, SharedHash, SharedEqual>> cache(4);

MyData d(new std::string("foo"));
MyKey k(new std::string("barlol"));
// Verfify start conditions.
EXPECT_TRUE(cache.size() == 0);
EXPECT_TRUE(d.use_count() == 1);
EXPECT_TRUE(k.use_count() == 1);
MyData d(std::make_shared<std::string>("foo"));
MyKey k(std::make_shared<std::string>("barlol"));
// Verify start conditions.
EXPECT_EQ(cache.size(), 0);
EXPECT_EQ(d.use_count(), 1);
EXPECT_EQ(k.use_count(), 1);
cache.insert(k, d);
EXPECT_TRUE(d.use_count() == 2);
EXPECT_TRUE(k.use_count() == 2);
EXPECT_EQ(d.use_count(), 2);
EXPECT_EQ(k.use_count(), 2);
cache.erase(k);
EXPECT_TRUE(d.use_count() == 1);
EXPECT_TRUE(k.use_count() == 1);
EXPECT_EQ(d.use_count(), 1);
EXPECT_EQ(k.use_count(), 1);
}

TEST("testCacheIterator") {
using Cache = lrucache_map< LruParam<int, std::string> >;
TEST(LruCacheMapTest, cache_iterator) {
using Cache = lrucache_map<LruParam<int, std::string>>;
Cache cache(3);
cache.insert(1, "first");
cache.insert(2, "second");
cache.insert(3, "third");
Cache::iterator it(cache.begin());
Cache::iterator mt(cache.end());
ASSERT_TRUE(it != mt);
ASSERT_EQUAL("third", *it);
ASSERT_EQ("third", *it);
ASSERT_TRUE(it != mt);
ASSERT_EQUAL("second", *(++it));
ASSERT_EQ("second", *(++it));
ASSERT_TRUE(it != mt);
ASSERT_EQUAL("second", *it++);
ASSERT_EQ("second", *it++);
ASSERT_TRUE(it != mt);
ASSERT_EQUAL("first", *it);
ASSERT_EQ("first", *it);
ASSERT_TRUE(it != mt);
it++;
++it;
ASSERT_TRUE(it == mt);
cache.insert(4, "fourth");
Cache::iterator it2(cache.begin());
Cache::iterator it3(cache.begin());
ASSERT_EQUAL("fourth", *it2);
ASSERT_EQ("fourth", *it2);
ASSERT_TRUE(it2 == it3);
it2++;
++it2;
ASSERT_TRUE(it2 != it3);
it2++;
it2++;
++it2;
++it2;
ASSERT_TRUE(it2 == mt);
Cache::iterator it4 = cache.erase(it3);
ASSERT_EQUAL("third", *it4);
ASSERT_EQUAL("third", *cache.begin());
ASSERT_EQ("third", *it4);
ASSERT_EQ("third", *cache.begin());
Cache::iterator it5(cache.erase(cache.end()));
ASSERT_TRUE(it5 == cache.end());
}

TEST("testCacheIteratorErase") {
using Cache = lrucache_map< LruParam<int, std::string> >;
namespace {

template <typename C>
std::string lru_key_order(C& cache) {
std::string keys;
for (auto it = cache.begin(); it != cache.end(); ++it) {
if (!keys.empty()) {
keys += ' ';
}
keys += std::to_string(it.key());
}
return keys;
}

}

TEST(LruCacheMapTest, cache_erase_by_iterator) {
using Cache = lrucache_map<LruParam<int, std::string>>;
Cache cache(3);
cache.insert(1, "first");
cache.insert(8, "second");
cache.insert(15, "third");
cache.insert(15, "third");
cache.insert(8, "second");
cache.insert(1, "first");
EXPECT_EQ(lru_key_order(cache), "1 8 15");
Cache::iterator it(cache.begin());
ASSERT_EQUAL("first", *it);
it++;
ASSERT_EQUAL("second", *it);
ASSERT_EQ("first", *it);
++it;
ASSERT_EQ("second", *it);
it = cache.erase(it);
ASSERT_EQUAL("third", *it);
EXPECT_EQ(lru_key_order(cache), "1 15");
ASSERT_EQ("third", *it);
cache.erase(it);
EXPECT_EQ(lru_key_order(cache), "1");
}

namespace {
TEST(LruCacheMapTest, find_no_ref_returns_iterator_if_present_and_does_not_update_lru) {
using Cache = lrucache_map<LruParam<int, std::string>>;
Cache cache(3);
cache.insert(1, "ichi");
cache.insert(2, "ni");
cache.insert(3, "san");
EXPECT_EQ(lru_key_order(cache), "3 2 1");

template <typename C>
std::string lru_key_order(C& cache) {
std::string keys;
for (auto it = cache.begin(); it != cache.end(); ++it) {
keys += std::to_string(it.key());
keys += ' ';
}
return keys;
}
auto iter = cache.find_no_ref(1);
ASSERT_TRUE(iter != cache.end());
EXPECT_EQ(*iter, "ichi");
EXPECT_EQ(lru_key_order(cache), "3 2 1");

iter = cache.find_no_ref(2);
ASSERT_TRUE(iter != cache.end());
EXPECT_EQ(*iter, "ni");
EXPECT_EQ(lru_key_order(cache), "3 2 1");

iter = cache.find_no_ref(4);
ASSERT_TRUE(iter == cache.end());
EXPECT_EQ(lru_key_order(cache), "3 2 1");
}

TEST("find_and_lazy_ref elides updating LRU head when less than half used") {
TEST(LruCacheMapTest, find_and_lazy_ref_elides_updating_LRU_head_when_less_than_half_full) {
using Cache = lrucache_map<LruParam<int, std::string>>;
Cache cache(6);
cache.insert(1, "a");
cache.insert(2, "b");
EXPECT_EQUAL(lru_key_order(cache), "2 1 ");
EXPECT_NOT_EQUAL(cache.find_and_lazy_ref(1), nullptr);
EXPECT_EQUAL(lru_key_order(cache), "2 1 "); // Not updated
EXPECT_EQ(lru_key_order(cache), "2 1");
EXPECT_NE(cache.find_and_lazy_ref(1), nullptr);
EXPECT_EQ(lru_key_order(cache), "2 1"); // Not updated
cache.insert(3, "c");
EXPECT_EQUAL(lru_key_order(cache), "3 2 1 ");
EXPECT_NOT_EQUAL(cache.find_and_lazy_ref(1), nullptr);
EXPECT_EQUAL(lru_key_order(cache), "3 2 1 "); // Still not > capacity/2
EXPECT_EQ(lru_key_order(cache), "3 2 1");
EXPECT_NE(cache.find_and_lazy_ref(1), nullptr);
EXPECT_EQ(lru_key_order(cache), "3 2 1"); // Still not > capacity/2
cache.insert(4, "c");
EXPECT_EQUAL(lru_key_order(cache), "4 3 2 1 ");
EXPECT_NOT_EQUAL(cache.find_and_lazy_ref(1), nullptr);
EXPECT_EQUAL(lru_key_order(cache), "1 4 3 2 "); // At long last, our time to LRU shine
EXPECT_EQ(lru_key_order(cache), "4 3 2 1");
EXPECT_NE(cache.find_and_lazy_ref(1), nullptr);
EXPECT_EQ(lru_key_order(cache), "1 4 3 2"); // At long last, our time to LRU shine
EXPECT_EQ(cache.find_and_lazy_ref(5), nullptr); // Key not found
EXPECT_EQ(lru_key_order(cache), "1 4 3 2");
}

TEST("Eager find_and_ref always moves to LRU head") {
TEST(LruCacheMapTest, eager_find_and_ref_always_moves_to_LRU_head) {
using Cache = lrucache_map<LruParam<int, std::string>>;
Cache cache(6);
cache.insert(1, "a");
Expand All @@ -228,13 +258,15 @@ TEST("Eager find_and_ref always moves to LRU head") {
cache.insert(4, "d");
cache.insert(5, "e");
cache.insert(6, "f");
EXPECT_EQUAL(lru_key_order(cache), "6 5 4 3 2 1 ");
EXPECT_NOT_EQUAL(cache.find_and_ref(2), nullptr);
EXPECT_EQUAL(lru_key_order(cache), "2 6 5 4 3 1 ");
EXPECT_NOT_EQUAL(cache.find_and_ref(5), nullptr);
EXPECT_EQUAL(lru_key_order(cache), "5 2 6 4 3 1 ");
EXPECT_NOT_EQUAL(cache.find_and_ref(1), nullptr);
EXPECT_EQUAL(lru_key_order(cache), "1 5 2 6 4 3 ");
EXPECT_EQ(lru_key_order(cache), "6 5 4 3 2 1");
EXPECT_NE(cache.find_and_ref(2), nullptr);
EXPECT_EQ(lru_key_order(cache), "2 6 5 4 3 1");
EXPECT_NE(cache.find_and_ref(5), nullptr);
EXPECT_EQ(lru_key_order(cache), "5 2 6 4 3 1");
EXPECT_NE(cache.find_and_ref(1), nullptr);
EXPECT_EQ(lru_key_order(cache), "1 5 2 6 4 3");
EXPECT_EQ(cache.find_and_ref(7), nullptr); // Key not found; no touching the shiny happy LRU
EXPECT_EQ(lru_key_order(cache), "1 5 2 6 4 3");
}

TEST_MAIN() { TEST_RUN_ALL(); }
GTEST_MAIN_RUN_ALL_TESTS()
12 changes: 9 additions & 3 deletions vespalib/src/vespa/vespalib/stllike/cache.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,18 @@ class cache {
[[nodiscard]] bool has_key(const KeyT& key) const noexcept;
// Precondition: key does not already exist in the mapping
void insert_and_update_size(const KeyT& key, ValueT value);
// Precondition: key must exist in the mapping
void erase_and_update_size(const KeyT& key);
void replace_and_update_size(const KeyT& key, ValueT value);
// Fetches an existing key from the cache _without_ updating the LRU ordering.
[[nodiscard]] const typename P::Value& get_existing(const KeyT& key) const;

// Returns true iff `key` existed in the mapping prior to the call, which also
// implies the mapping has been updated by consuming `value` (i.e. its contents
// has been std::move()'d away and it is now in a logically empty state).
// Otherwise, both the mapping and `value` remains unmodified and false is returned.
[[nodiscard]] bool try_replace_and_update_size(const KeyT& key, ValueT& value);
// Iff `key` was present in the mapping prior to the call, its entry is removed
// from the mapping and true is returned. Otherwise, the mapping remains unmodified
// and false is returned.
[[nodiscard]] bool try_erase_and_update_size(const KeyT& key);
// Iff element exists in cache, assigns `val_out` the stored value and returns true.
// This also updates the underlying LRU order.
// Otherwise, `val_out` is not modified and false is returned.
Expand Down
Loading

0 comments on commit 239bc8d

Please sign in to comment.