From accba097094e6665159a848cc870d88090df45a6 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Mon, 27 Sep 2021 15:41:53 -0700 Subject: [PATCH] Remove entry_was_evicted from LRU This was no longer used. This allows us to remove the CRTP bits as well. --- src/autoload.cpp | 2 +- src/fish_tests.cpp | 11 +++-------- src/history.cpp | 5 ++--- src/lru.h | 32 ++++++++------------------------ 4 files changed, 14 insertions(+), 36 deletions(-) diff --git a/src/autoload.cpp b/src/autoload.cpp index c686e3542..156c13f0a 100644 --- a/src/autoload.cpp +++ b/src/autoload.cpp @@ -35,7 +35,7 @@ class autoload_file_cache_t { /// Our LRU cache of checks that were misses. /// The key is the command, the value is the time of the check. - struct misses_lru_cache_t : public lru_cache_t {}; + using misses_lru_cache_t = lru_cache_t; misses_lru_cache_t misses_cache_; /// The set of files that we have returned to the caller, along with the time of the check. diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index b48c80029..3d71877de 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -1995,16 +1995,12 @@ static void test_escape_sequences() { err(L"test_escape_sequences failed on line %d\n", __LINE__); } -class test_lru_t : public lru_cache_t { +class test_lru_t : public lru_cache_t { public: static constexpr size_t test_capacity = 16; using value_type = std::pair; - test_lru_t() : lru_cache_t(test_capacity) {} - - std::vector evicted; - - void entry_was_evicted(const wcstring &key, int val) { evicted.emplace_back(key, val); } + test_lru_t() : lru_cache_t(test_capacity) {} std::vector values() const { std::vector result; @@ -2044,7 +2040,6 @@ static void test_lru() { } cache.check_sanity(); } - do_test(cache.evicted == expected_evicted); do_test(cache.values() == expected_values); cache.check_sanity(); @@ -2071,7 +2066,7 @@ static void test_lru() { } cache.evict_all_nodes(); - do_test(cache.evicted.size() == size_t(total_nodes)); + do_test(cache.size() == 0); } /// An environment built around an std::map. diff --git a/src/history.cpp b/src/history.cpp index 2a36c1814..b1cbad749 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -125,10 +125,9 @@ maybe_t history_filename(const wcstring &session_id, const wcstring &s } // anonymous namespace -class history_lru_cache_t : public lru_cache_t { +class history_lru_cache_t : public lru_cache_t { public: - explicit history_lru_cache_t(size_t max) - : lru_cache_t(max) {} + explicit history_lru_cache_t(size_t max) : lru_cache_t(max) {} /// Function to add a history item. void add_item(history_item_t item) { diff --git a/src/lru.h b/src/lru.h index f6eac3265..1817355cc 100644 --- a/src/lru.h +++ b/src/lru.h @@ -10,15 +10,14 @@ // Least-recently-used cache class. // // This a map from wcstring to Contents, that will evict entries when the count exceeds the maximum. -// It uses CRTP to inform clients when entries are evicted. This uses the classic LRU cache -// structure: a dictionary mapping keys to nodes, where the nodes also form a linked list. Our -// linked list is circular and has a sentinel node (the "mouth" - picture a snake swallowing its -// tail). This simplifies the logic: no pointer is ever NULL! It also works well with C++'s iterator -// since the sentinel node is a natural value for end(). Our nodes also have the unusual property of -// having a "back pointer": they store an iterator to the entry in the map containing the node. This -// allows us, given a node, to immediately locate the node and its key in the dictionary. This -// allows us to avoid duplicating the key in the node. -template +// This uses the classic LRU cache structure: a dictionary mapping keys to nodes, where the nodes +// also form a linked list. Our linked list is circular and has a sentinel node (the "mouth" - +// picture a snake swallowing its tail). This simplifies the logic: no pointer is ever NULL! It also +// works well with C++'s iterator since the sentinel node is a natural value for end(). Our nodes +// also have the unusual property of having a "back pointer": they store an iterator to the entry in +// the map containing the node. This allows us, given a node, to immediately locate the node and its +// key in the dictionary. This allows us to avoid duplicating the key in the node. +template class lru_cache_t { struct lru_node_t; struct lru_link_t : noncopyable_t { @@ -80,17 +79,8 @@ class lru_cache_t { node->prev->next = node->next; node->next->prev = node->prev; - // Pull out our key and value - // Note we copy the key in case the map needs it to erase the node - wcstring key = *node->key; - Contents value(std::move(node->value)); - // Remove us from the map. This deallocates node! node_map.erase(iter); - - // Tell ourselves what we did - Derived *dthis = static_cast(this); - dthis->entry_was_evicted(std::move(key), std::move(value)); } // Evicts the last node @@ -99,12 +89,6 @@ class lru_cache_t { evict_node(static_cast(mouth.prev)); } - // CRTP callback for when a node is evicted. - // Clients can implement this - void entry_was_evicted(const wcstring &key, Contents value) { - UNUSED(key); - UNUSED(value); - } // Implementation of merge step for mergesort. // Given two singly linked lists left and right, and a binary func F implementing less-than,