From ba7be7fb774d665432b7924abe24e2315ad95459 Mon Sep 17 00:00:00 2001 From: haochengxia Date: Wed, 30 Sep 2026 09:54:49 -0500 Subject: [PATCH 1/5] Test the registry lookup, register MRU and LRUv0, document case-insensitive names The registry this PR originally proposed landed in #324, so what is left is the part around it. Rebased onto develop after the docs rewrite (#372) and the LRUv0 fix (#373). test_evictionAlgo gains a case for create_cache_using_plugin(). Nothing exercised that entry point, which is the one the MINISIM profiler uses and the one that failed in issue #304: it went straight to dlsym(), which cannot see a built-in constructor in a statically linked build because the archive member holding it is never pulled in. The case also pins the case-insensitive matching, since MINISIM callers pass names in whatever case the user typed. MRU and LRUv0 are compiled into the library but were absent from the registry, so no CLI could reach them. LRUv0 was left out earlier because it segfaulted on the first request; #373 fixed that, so it is registered now. Both names are covered by the new test. The SHARDS profiler compared both the profiler name and the algorithm with strcmp(), so --algo=lru was rejected even though MINISIM and cachesim accept it. It uses strcasecmp() now, which makes "names are case-insensitive" true for every mrcProfiler path, as the docs now say. Docs: the extend guide says names are matched case-insensitively and when an algorithm also needs a branch in cache_init.h, the mrcProfiler option table marks --algo as case-insensitive, scripts/README.md says the same for --algos, and the README lists mru. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015VD7L6jWd2YzyhfMJxMquc --- README.md | 2 +- doc/advanced_lib_extend.md | 2 +- doc/quickstart_mrcProfiler.md | 2 +- libCacheSim/bin/mrcProfiler/cli_parser.cpp | 10 +++---- libCacheSim/cache/cacheAlgoRegistry.c | 2 ++ scripts/README.md | 1 + test/test_evictionAlgo.c | 31 ++++++++++++++++++++++ 7 files changed, 42 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 9a605dac5..9a84f253b 100644 --- a/README.md +++ b/README.md @@ -339,7 +339,7 @@ See [libCacheSim-node](/libCacheSim-node/) for the full API and the bundled `cac The name in `code` is what you pass to `cachesim` (names are case-insensitive). ### Eviction algorithms -* [FIFO](/libCacheSim/cache/eviction/FIFO.c) `fifo`, [LRU](/libCacheSim/cache/eviction/LRU.c) `lru`, [Clock](/libCacheSim/cache/eviction/Clock.c) `clock`, [SLRU](/libCacheSim/cache/eviction/SLRU.c) `slru`, [Random](/libCacheSim/cache/eviction/Random.c) `random`, [RandomTwo](/libCacheSim/cache/eviction/RandomTwo.c) `randomtwo` +* [FIFO](/libCacheSim/cache/eviction/FIFO.c) `fifo`, [LRU](/libCacheSim/cache/eviction/LRU.c) `lru`, [MRU](/libCacheSim/cache/eviction/MRU.c) `mru`, [Clock](/libCacheSim/cache/eviction/Clock.c) `clock`, [SLRU](/libCacheSim/cache/eviction/SLRU.c) `slru`, [Random](/libCacheSim/cache/eviction/Random.c) `random`, [RandomTwo](/libCacheSim/cache/eviction/RandomTwo.c) `randomtwo` * [LFU](/libCacheSim/cache/eviction/LFU.c) `lfu`, [LFU with dynamic aging](/libCacheSim/cache/eviction/LFUDA.c) `lfuda` * [ARC](/libCacheSim/cache/eviction/ARC.c) `arc`, [TwoQ](/libCacheSim/cache/eviction/TwoQ.c) `2q`, [MQ](/libCacheSim/cache/eviction/MQ.c) `mq`, [CLOCK-PRO](/libCacheSim/cache/eviction/ClockPro.c) `clockpro`, [CAR](/libCacheSim/cache/eviction/CAR.c) `car`, [LIRS](/libCacheSim/cache/eviction/LIRS.c) `lirs`, [Clock2QPlus](/libCacheSim/cache/eviction/Clock2QPlus.c) `clock2qplus` * [LRU-K](/libCacheSim/cache/eviction/cpp/LRU_K.cpp) `lru-k`, [LRU-Prob](/libCacheSim/cache/eviction/LRUProb.c) `lru-prob`, [Size](/libCacheSim/cache/eviction/Size.c) `size` diff --git a/doc/advanced_lib_extend.md b/doc/advanced_lib_extend.md index 204266ac1..f13b48bb4 100644 --- a/doc/advanced_lib_extend.md +++ b/doc/advanced_lib_extend.md @@ -53,7 +53,7 @@ Then wire it in. Using `myCache` as the example name: 2. **Add per-object metadata, if you need it.** Add a struct for it to the union in [include/libCacheSim/cacheObj.h](/libCacheSim/include/libCacheSim/cacheObj.h). 3. **Declare the constructor.** Add `myCache_init()` to [include/libCacheSim/evictionAlgo.h](/libCacheSim/include/libCacheSim/evictionAlgo.h). 4. **Compile it.** Add `eviction/myCache.c` to [cache/CMakeLists.txt](/libCacheSim/cache/CMakeLists.txt). -5. **Give it a name.** Add `{"mycache", myCache_init}` to the table in [cache/cacheAlgoRegistry.c](/libCacheSim/cache/cacheAlgoRegistry.c). The table is alphabetical. This one entry makes the algorithm available to `cachesim`, to the MINISIM mode of `mrcProfiler`, and to `create_cache_by_name()`. +5. **Give it a name.** Add `{"mycache", myCache_init}` to the table in [cache/cacheAlgoRegistry.c](/libCacheSim/cache/cacheAlgoRegistry.c). The table is alphabetical. This one entry makes the algorithm available to `cachesim`, to the MINISIM mode of `mrcProfiler`, and to `create_cache_by_name()`. Names are matched case-insensitively, so users can type `mycache` or `myCache`. Only an algorithm that needs more than a lookup, such as a smaller hash table or a check that the trace carries future access times, also needs a branch in [bin/cachesim/cache_init.h](/libCacheSim/bin/cachesim/cache_init.h). 6. **Test and document it.** Add a test to [test/test_evictionAlgo.c](/test/test_evictionAlgo.c), and list the algorithm in the [README](/README.md#supported-algorithms) with the name users type. If it takes `-e` parameters, add a case to the `testCLI` target too, as described in [CONTRIBUTING.md](/CONTRIBUTING.md#testing). Then rebuild and try it: diff --git a/doc/quickstart_mrcProfiler.md b/doc/quickstart_mrcProfiler.md index bfff77aeb..9bc058e26 100644 --- a/doc/quickstart_mrcProfiler.md +++ b/doc/quickstart_mrcProfiler.md @@ -49,7 +49,7 @@ Each row is one cache size. When you give sizes as fractions, the first column i | Option | Meaning | | --- | --- | -| `--algo` | Eviction algorithm. SHARDS only supports `LRU`. MINISIM accepts any name `cachesim` accepts. | +| `--algo` | Eviction algorithm, case-insensitive (`fifo` and `FIFO` are the same). SHARDS only supports `LRU`. MINISIM accepts any name `cachesim` accepts. | | `--profiler` | `SHARDS` or `MINISIM` | | `--profiler-params` | See [Profiler parameters](#profiler-parameters) | | `--size` | See [Choose the cache sizes](#choose-the-cache-sizes) | diff --git a/libCacheSim/bin/mrcProfiler/cli_parser.cpp b/libCacheSim/bin/mrcProfiler/cli_parser.cpp index 18e810bbb..543276a8f 100644 --- a/libCacheSim/bin/mrcProfiler/cli_parser.cpp +++ b/libCacheSim/bin/mrcProfiler/cli_parser.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include @@ -370,10 +371,10 @@ void mrc_profiler_params_parse(const char *cache_algorithm_str, mrcProfiler::mrc_profiler_e &profiler_type, mrcProfiler::mrc_profiler_params_t ¶ms) { // initial the params of mrc profiler - if (strcmp(profiler_str, "SHARDS") == 0 || - strcmp(profiler_str, "shards") == 0) { + // names are case-insensitive, the same as cachesim and the MINISIM registry + if (strcasecmp(profiler_str, "SHARDS") == 0) { profiler_type = mrcProfiler::SHARDS_PROFILER; - if (strcmp(cache_algorithm_str, "LRU")) { + if (strcasecmp(cache_algorithm_str, "LRU") != 0) { ERROR("cache algorithm must be LRU for SHARDS\n") exit(1); } @@ -382,8 +383,7 @@ void mrc_profiler_params_parse(const char *cache_algorithm_str, // init shards params params.shards_params.parse_params(params_str); - } else if (strcmp(profiler_str, "MINISIM") == 0 || - strcmp(profiler_str, "minisim") == 0) { + } else if (strcasecmp(profiler_str, "MINISIM") == 0) { profiler_type = mrcProfiler::MINISIM_PROFILER; params.cache_algorithm_str = (char *)cache_algorithm_str; diff --git a/libCacheSim/cache/cacheAlgoRegistry.c b/libCacheSim/cache/cacheAlgoRegistry.c index bb70f9332..e2814de2d 100644 --- a/libCacheSim/cache/cacheAlgoRegistry.c +++ b/libCacheSim/cache/cacheAlgoRegistry.c @@ -53,8 +53,10 @@ static const cache_algo_entry_t g_cache_algos[] = { {"lru", LRU_init}, {"lru-k", LRU_K_init}, {"lru-prob", LRU_Prob_init}, + {"lruv0", LRUv0_init}, {"merlin", merlin_init}, {"mq", MQ_init}, + {"mru", MRU_init}, {"multiqueue", MQ_init}, {"nop", nop_init}, /* plugin cache that allows user to implement custom cache */ diff --git a/scripts/README.md b/scripts/README.md index 5775bf550..5fe988ae4 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -28,6 +28,7 @@ python3 plot_mrc_time.py \ ## Profile miss ratio curves `mrcProfiler` enables fast profiling of miss ratio curves. With a proper parameter setting (such as `sampling_rate`, see [here](../doc/quickstart_mrcProfiler.md) for details) `mrcProfiler` can profile miss ratio curves in a fast way with acceptable accuracy. +Names passed to `--algos` are case-insensitive, so `LRU,FIFO` and `lru,fifo` both work. ```bash # plot profiled miss ratio over sizes diff --git a/test/test_evictionAlgo.c b/test/test_evictionAlgo.c index 331e94392..656582e5f 100644 --- a/test/test_evictionAlgo.c +++ b/test/test_evictionAlgo.c @@ -563,6 +563,34 @@ static void test_WTinyLFU(gconstpointer user_data) { // TODO: to be implemented } +/* The MINISIM profiler only knows an algorithm by its name and reaches it + * through create_cache_using_plugin(). That used to go straight to dlsym(), + * which cannot see a built-in constructor in a statically linked build — the + * archive member holding it is never pulled in — so every built-in name failed + * with "undefined symbol: lru_init" (issue #304). The registry lookup that + * replaced it matches names case-insensitively, the same way cachesim does. */ +static void test_create_cache_by_name(gconstpointer user_data) { + common_cache_params_t cc_params = { + .cache_size = CACHE_SIZE, .hashpower = 20, .default_ttl = DEFAULT_TTL}; + + static const struct { + const char *requested; + const char *expected_name; + } names[] = { + {"lru", "LRU"}, {"LRU", "LRU"}, {"Lru", "LRU"}, {"fifo", "FIFO"}, + {"FIFO", "FIFO"}, {"fIfO", "FIFO"}, {"mru", "MRU"}, {"MRU", "MRU"}, + {"lruv0", "LRUv0"}, {"LRUv0", "LRUv0"}, + }; + + for (size_t i = 0; i < sizeof(names) / sizeof(names[0]); i++) { + cache_t *cache = + create_cache_using_plugin(names[i].requested, cc_params, NULL); + g_assert_nonnull(cache); + g_assert_cmpstr(cache->cache_name, ==, names[i].expected_name); + cache->cache_free(cache); + } +} + static void empty_test(gconstpointer user_data) { ; } static void test_WorkloadAware(gconstpointer user_data) { @@ -646,6 +674,9 @@ int main(int argc, char *argv[]) { g_test_add_data_func("/libCacheSim/cacheAlgo_BeladySize", reader, test_BeladySize); + g_test_add_data_func("/libCacheSim/create_cache_by_name", reader, + test_create_cache_by_name); + g_test_add_data_func_full("/libCacheSim/empty", reader, empty_test, test_teardown); From 41845879d864002987dd4abc964b8de1c07be80f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 02:21:00 +0000 Subject: [PATCH 2/5] fix(MRU): do not write through a NULL head when evicting the last object MRU_evict advanced q_head and then wrote q_head->queue.prev unconditionally. When the evicted object was the only one in the queue, the new head is NULL, so the second miss in a one-object cache (or any eviction that empties the cache, which variable-sized traces can need) segfaulted. q_tail was also left pointing at the freed object. Handle the empty-queue case the way LRU_evict does: clear q_tail when the head runs out. Registering mru in the previous commit is what makes this path reachable from cachesim and the MRC profiler, so fix it here. The new test drives a one-object MRU cache through four misses; it segfaults without the fix and passes with it, including under ASan and LeakSanitizer. Miss ratios on the sample trace are unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015VD7L6jWd2YzyhfMJxMquc --- libCacheSim/cache/eviction/MRU.c | 10 ++++++++-- test/test_evictionAlgo.c | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/libCacheSim/cache/eviction/MRU.c b/libCacheSim/cache/eviction/MRU.c index 8c018586f..5a1520624 100644 --- a/libCacheSim/cache/eviction/MRU.c +++ b/libCacheSim/cache/eviction/MRU.c @@ -177,8 +177,14 @@ static cache_obj_t *MRU_to_evict(cache_t *cache, const request_t *req) { static void MRU_evict(cache_t *cache, const request_t *req) { MRU_params_t *params = (MRU_params_t *)cache->eviction_params; cache_obj_t *obj_to_evict = MRU_to_evict(cache, req); - params->q_head = params->q_head->queue.next; - params->q_head->queue.prev = NULL; + DEBUG_ASSERT(obj_to_evict != NULL); + params->q_head = obj_to_evict->queue.next; + if (likely(params->q_head != NULL)) { + params->q_head->queue.prev = NULL; + } else { + /* that was the only object, so the queue is now empty */ + params->q_tail = NULL; + } cache_evict_base(cache, obj_to_evict, true); } diff --git a/test/test_evictionAlgo.c b/test/test_evictionAlgo.c index 656582e5f..e672f70cc 100644 --- a/test/test_evictionAlgo.c +++ b/test/test_evictionAlgo.c @@ -419,6 +419,28 @@ static void test_MRU(gconstpointer user_data) { test_cache_algorithm(user_data, &test_data_truth[18]); } +// Evicting the only object empties the queue. MRU_evict used to write through +// the new, NULL, head and leave the tail pointing at the freed object, so the +// second miss in a one-object cache segfaulted. +static void test_MRU_evict_only_object(void) { + common_cache_params_t cc_params = {.cache_size = 1, .hashpower = 8}; + cache_t *cache = MRU_init(cc_params, NULL); + request_t *req = new_request(); + req->obj_size = 1; + + for (obj_id_t id = 1; id <= 4; id++) { + req->obj_id = id; + g_assert_false(cache->get(cache, req)); + g_assert_cmpint(cache->get_n_obj(cache), ==, 1); + g_assert_nonnull(cache->find(cache, req, false)); + req->obj_id = id - 1; + g_assert_null(cache->find(cache, req, false)); + } + + free_request(req); + cache->cache_free(cache); +} + static void test_QDLP_FIFO(gconstpointer user_data) { test_cache_algorithm(user_data, &test_data_truth[19]); } @@ -639,6 +661,8 @@ int main(int argc, char *argv[]) { g_test_add_data_func("/libCacheSim/cacheAlgo_Merlin", reader, test_Merlin); g_test_add_data_func("/libCacheSim/cacheAlgo_MQ", reader, test_MQ); g_test_add_data_func("/libCacheSim/cacheAlgo_MRU", reader, test_MRU); + g_test_add_func("/libCacheSim/cacheAlgo_MRU_evict_only_object", + test_MRU_evict_only_object); g_test_add_data_func("/libCacheSim/cacheAlgo_QDLP_FIFO", reader, test_QDLP_FIFO); g_test_add_data_func("/libCacheSim/cacheAlgo_Random", reader, test_Random); From 45fc5e1121722001e5a43832cf650227afef2d45 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 02:27:29 +0000 Subject: [PATCH 3/5] fix(LRUv0): go through cache_get_base so admission policies apply LRUv0_get inserted every miss itself instead of calling cache_get_base, so it never asked can_insert. With lruv0 now reachable from the CLI, -a was silently ignored: on cloudPhysicsIO at 0.1 with -a size and size=4096, LRU's miss ratio is 0.9258 while LRUv0 stayed at its no-admission 0.8051. The custom get also skipped the prefetcher and never counted requests. LRUv0 already sets find, insert and evict, which are what cache_get_base calls, so LRUv0_get now just calls it, as LRU does. With size admission LRUv0 now matches LRU at 0.9258, its miss ratios without admission are unchanged, and the existing LRUv0 test still matches LRU's expected results. The new test attaches a size admissioner and checks that an object over the threshold is never cached; it fails without the fix and passes with it, including under ASan and LeakSanitizer. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015VD7L6jWd2YzyhfMJxMquc --- libCacheSim/cache/eviction/LRUv0.c | 15 +++++---------- test/test_evictionAlgo.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/libCacheSim/cache/eviction/LRUv0.c b/libCacheSim/cache/eviction/LRUv0.c index cb5a42d89..5db1373be 100644 --- a/libCacheSim/cache/eviction/LRUv0.c +++ b/libCacheSim/cache/eviction/LRUv0.c @@ -113,21 +113,16 @@ static void LRUv0_free(cache_t *cache) { * return false * ``` * + * cache_get_base does this through the find, insert and evict hooks set in + * LRUv0_init. It also asks can_insert before inserting, so an admission policy + * given with -a is honored, and it runs the prefetcher and counts requests. + * * @param cache * @param req * @return true if cache hit, false if cache miss */ static bool LRUv0_get(cache_t *cache, const request_t *req) { - bool cache_hit = LRUv0_find(cache, req, true); - if (req->obj_size <= cache->cache_size) { - if (!cache_hit) LRUv0_insert(cache, req); - - while (cache->occupied_byte > cache->cache_size) LRUv0_evict(cache, req); - } else { - WARN("req %lld: obj size %ld larger than cache size %ld\n", - (long long)req->obj_id, (long)req->obj_size, (long)cache->cache_size); - } - return cache_hit; + return cache_get_base(cache, req); } // *********************************************************************** diff --git a/test/test_evictionAlgo.c b/test/test_evictionAlgo.c index e672f70cc..3781dfaad 100644 --- a/test/test_evictionAlgo.c +++ b/test/test_evictionAlgo.c @@ -406,6 +406,32 @@ static void test_LRUv0(gconstpointer user_data) { test_cache_algorithm(user_data, &test_data); } +// LRUv0_get used to insert every miss itself, skipping cache_get_base and with +// it the admission check, so -a was silently ignored for lruv0. +static void test_LRUv0_honors_admission(void) { + common_cache_params_t cc_params = {.cache_size = 1000, .hashpower = 8}; + cache_t *cache = LRUv0_init(cc_params, NULL); + cache->admissioner = create_admissioner("size", "size=100"); + request_t *req = new_request(); + + // too large for the admission policy, though it fits in the cache + req->obj_id = 1; + req->obj_size = 200; + g_assert_false(cache->get(cache, req)); + g_assert_false(cache->get(cache, req)); + g_assert_cmpint(cache->get_n_obj(cache), ==, 0); + + // small enough to be admitted + req->obj_id = 2; + req->obj_size = 50; + g_assert_false(cache->get(cache, req)); + g_assert_true(cache->get(cache, req)); + g_assert_cmpint(cache->get_n_obj(cache), ==, 1); + + free_request(req); + cache->cache_free(cache); +} + static void test_LRU_K(gconstpointer user_data) { test_cache_algorithm(user_data, &test_data_truth[17]); } @@ -657,6 +683,8 @@ int main(int argc, char *argv[]) { g_test_add_data_func("/libCacheSim/cacheAlgo_LIRS", reader, test_LIRS); g_test_add_data_func("/libCacheSim/cacheAlgo_LRU", reader, test_LRU); g_test_add_data_func("/libCacheSim/cacheAlgo_LRUv0", reader, test_LRUv0); + g_test_add_func("/libCacheSim/cacheAlgo_LRUv0_honors_admission", + test_LRUv0_honors_admission); g_test_add_data_func("/libCacheSim/cacheAlgo_LRU_K", reader, test_LRU_K); g_test_add_data_func("/libCacheSim/cacheAlgo_Merlin", reader, test_Merlin); g_test_add_data_func("/libCacheSim/cacheAlgo_MQ", reader, test_MQ); From 4ffe9ffa7f781cb4d7efccb12fe12d4dc8257a86 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 02:31:16 +0000 Subject: [PATCH 4/5] fix(LRUv0): still evict after a hit that grows an object The previous commit routed LRUv0_get through cache_get_base, which only evicts on a miss. LRUv0_find, unlike LRU's, adopts the larger size when a hit carries one, and the old LRUv0_get then evicted until the cache was back under its size. Without that loop a 100-byte cache given (1,60), (2,40), (1,80) held 120 bytes and still hit on object 2. Keep cache_get_base for admission, prefetching and request counting, and run the eviction loop after it again. LRUv0's miss ratios on cloudPhysicsIO now match develop's exactly at 0.01, 0.05, 0.1, 0.3 and 0.6 without admission, and it still honors -a. The new test replays that sequence; it fails on the previous commit and passes now, including under ASan and LeakSanitizer. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015VD7L6jWd2YzyhfMJxMquc --- libCacheSim/cache/eviction/LRUv0.c | 10 +++++++++- test/test_evictionAlgo.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/libCacheSim/cache/eviction/LRUv0.c b/libCacheSim/cache/eviction/LRUv0.c index 5db1373be..9f4af64b1 100644 --- a/libCacheSim/cache/eviction/LRUv0.c +++ b/libCacheSim/cache/eviction/LRUv0.c @@ -117,12 +117,20 @@ static void LRUv0_free(cache_t *cache) { * LRUv0_init. It also asks can_insert before inserting, so an admission policy * given with -a is honored, and it runs the prefetcher and counts requests. * + * Unlike LRU, LRUv0_find adopts the new size when a hit carries one, but + * cache_get_base only evicts on a miss. So evict here too when a hit grew an + * object past the cache size, as LRUv0 always has. + * * @param cache * @param req * @return true if cache hit, false if cache miss */ static bool LRUv0_get(cache_t *cache, const request_t *req) { - return cache_get_base(cache, req); + bool hit = cache_get_base(cache, req); + while (cache->occupied_byte > cache->cache_size && cache->n_obj > 0) { + LRUv0_evict(cache, req); + } + return hit; } // *********************************************************************** diff --git a/test/test_evictionAlgo.c b/test/test_evictionAlgo.c index 3781dfaad..d649e0e7d 100644 --- a/test/test_evictionAlgo.c +++ b/test/test_evictionAlgo.c @@ -432,6 +432,34 @@ static void test_LRUv0_honors_admission(void) { cache->cache_free(cache); } +// LRUv0 adopts a larger size when a hit carries one. The cache must then evict +// to get back under its size, even though the request was a hit. +static void test_LRUv0_evicts_when_hit_grows_object(void) { + common_cache_params_t cc_params = {.cache_size = 100, .hashpower = 8}; + cache_t *cache = LRUv0_init(cc_params, NULL); + request_t *req = new_request(); + + req->obj_id = 1; + req->obj_size = 60; + g_assert_false(cache->get(cache, req)); + req->obj_id = 2; + req->obj_size = 40; + g_assert_false(cache->get(cache, req)); + + // object 1 grows to 80 bytes, so object 2, now least recent, must go + req->obj_id = 1; + req->obj_size = 80; + g_assert_true(cache->get(cache, req)); + g_assert_cmpint(cache->get_occupied_byte(cache), <=, 100); + g_assert_cmpint(cache->get_n_obj(cache), ==, 1); + req->obj_id = 2; + req->obj_size = 40; + g_assert_null(cache->find(cache, req, false)); + + free_request(req); + cache->cache_free(cache); +} + static void test_LRU_K(gconstpointer user_data) { test_cache_algorithm(user_data, &test_data_truth[17]); } @@ -685,6 +713,8 @@ int main(int argc, char *argv[]) { g_test_add_data_func("/libCacheSim/cacheAlgo_LRUv0", reader, test_LRUv0); g_test_add_func("/libCacheSim/cacheAlgo_LRUv0_honors_admission", test_LRUv0_honors_admission); + g_test_add_func("/libCacheSim/cacheAlgo_LRUv0_evicts_when_hit_grows_object", + test_LRUv0_evicts_when_hit_grows_object); g_test_add_data_func("/libCacheSim/cacheAlgo_LRU_K", reader, test_LRU_K); g_test_add_data_func("/libCacheSim/cacheAlgo_Merlin", reader, test_Merlin); g_test_add_data_func("/libCacheSim/cacheAlgo_MQ", reader, test_MQ); From 38851c7a37385132c9d3c96bb35654c7a4739131 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 02:35:31 +0000 Subject: [PATCH 5/5] Leave lruv0 out of the registry again and revert the LRUv0 patches Registering lruv0 made the CLI hand it options its implementation never sees. LRUv0 is the older GLib-based LRU and keeps its own hash table, so it bypasses cache_find_base, cache_insert_base and cache_evict_base, and with them everything built on those paths: admission (45fc5e1 patched that), TTL expiry and default TTLs, and the prefetchers' find and evict callbacks, so OBL never turns prefetching on and PG never builds its graph. Each review round found another of these, and patching them one at a time does not converge; doing it properly means moving LRUv0 onto the shared paths, which is its own change. So go back to what the original PR did and leave lruv0 out, with a comment that gives the actual reason now that the crash is fixed (#373). Revert 45fc5e1 and 4ffe9ff, which only existed to make the registered lruv0 behave, so LRUv0.c is untouched by this branch. The lookup test now checks that lruv0 stays unregistered. This reverts commits 45fc5e1 and 4ffe9ff. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015VD7L6jWd2YzyhfMJxMquc --- libCacheSim/cache/cacheAlgoRegistry.c | 5 +- libCacheSim/cache/eviction/LRUv0.c | 21 ++++----- test/test_evictionAlgo.c | 66 ++------------------------- 3 files changed, 18 insertions(+), 74 deletions(-) diff --git a/libCacheSim/cache/cacheAlgoRegistry.c b/libCacheSim/cache/cacheAlgoRegistry.c index e2814de2d..a85f341fc 100644 --- a/libCacheSim/cache/cacheAlgoRegistry.c +++ b/libCacheSim/cache/cacheAlgoRegistry.c @@ -53,7 +53,10 @@ static const cache_algo_entry_t g_cache_algos[] = { {"lru", LRU_init}, {"lru-k", LRU_K_init}, {"lru-prob", LRU_Prob_init}, - {"lruv0", LRUv0_init}, + /* lruv0 is deliberately absent. LRUv0 is the older GLib-based LRU and keeps + * its own hash table, so it bypasses the shared find, insert and evict + * paths that implement TTL expiry, admission and the prefetcher callbacks. + * Naming it here would let the CLI silently ignore those options. */ {"merlin", merlin_init}, {"mq", MQ_init}, {"mru", MRU_init}, diff --git a/libCacheSim/cache/eviction/LRUv0.c b/libCacheSim/cache/eviction/LRUv0.c index 9f4af64b1..cb5a42d89 100644 --- a/libCacheSim/cache/eviction/LRUv0.c +++ b/libCacheSim/cache/eviction/LRUv0.c @@ -113,24 +113,21 @@ static void LRUv0_free(cache_t *cache) { * return false * ``` * - * cache_get_base does this through the find, insert and evict hooks set in - * LRUv0_init. It also asks can_insert before inserting, so an admission policy - * given with -a is honored, and it runs the prefetcher and counts requests. - * - * Unlike LRU, LRUv0_find adopts the new size when a hit carries one, but - * cache_get_base only evicts on a miss. So evict here too when a hit grew an - * object past the cache size, as LRUv0 always has. - * * @param cache * @param req * @return true if cache hit, false if cache miss */ static bool LRUv0_get(cache_t *cache, const request_t *req) { - bool hit = cache_get_base(cache, req); - while (cache->occupied_byte > cache->cache_size && cache->n_obj > 0) { - LRUv0_evict(cache, req); + bool cache_hit = LRUv0_find(cache, req, true); + if (req->obj_size <= cache->cache_size) { + if (!cache_hit) LRUv0_insert(cache, req); + + while (cache->occupied_byte > cache->cache_size) LRUv0_evict(cache, req); + } else { + WARN("req %lld: obj size %ld larger than cache size %ld\n", + (long long)req->obj_id, (long)req->obj_size, (long)cache->cache_size); } - return hit; + return cache_hit; } // *********************************************************************** diff --git a/test/test_evictionAlgo.c b/test/test_evictionAlgo.c index d649e0e7d..0a71a5d0c 100644 --- a/test/test_evictionAlgo.c +++ b/test/test_evictionAlgo.c @@ -406,60 +406,6 @@ static void test_LRUv0(gconstpointer user_data) { test_cache_algorithm(user_data, &test_data); } -// LRUv0_get used to insert every miss itself, skipping cache_get_base and with -// it the admission check, so -a was silently ignored for lruv0. -static void test_LRUv0_honors_admission(void) { - common_cache_params_t cc_params = {.cache_size = 1000, .hashpower = 8}; - cache_t *cache = LRUv0_init(cc_params, NULL); - cache->admissioner = create_admissioner("size", "size=100"); - request_t *req = new_request(); - - // too large for the admission policy, though it fits in the cache - req->obj_id = 1; - req->obj_size = 200; - g_assert_false(cache->get(cache, req)); - g_assert_false(cache->get(cache, req)); - g_assert_cmpint(cache->get_n_obj(cache), ==, 0); - - // small enough to be admitted - req->obj_id = 2; - req->obj_size = 50; - g_assert_false(cache->get(cache, req)); - g_assert_true(cache->get(cache, req)); - g_assert_cmpint(cache->get_n_obj(cache), ==, 1); - - free_request(req); - cache->cache_free(cache); -} - -// LRUv0 adopts a larger size when a hit carries one. The cache must then evict -// to get back under its size, even though the request was a hit. -static void test_LRUv0_evicts_when_hit_grows_object(void) { - common_cache_params_t cc_params = {.cache_size = 100, .hashpower = 8}; - cache_t *cache = LRUv0_init(cc_params, NULL); - request_t *req = new_request(); - - req->obj_id = 1; - req->obj_size = 60; - g_assert_false(cache->get(cache, req)); - req->obj_id = 2; - req->obj_size = 40; - g_assert_false(cache->get(cache, req)); - - // object 1 grows to 80 bytes, so object 2, now least recent, must go - req->obj_id = 1; - req->obj_size = 80; - g_assert_true(cache->get(cache, req)); - g_assert_cmpint(cache->get_occupied_byte(cache), <=, 100); - g_assert_cmpint(cache->get_n_obj(cache), ==, 1); - req->obj_id = 2; - req->obj_size = 40; - g_assert_null(cache->find(cache, req, false)); - - free_request(req); - cache->cache_free(cache); -} - static void test_LRU_K(gconstpointer user_data) { test_cache_algorithm(user_data, &test_data_truth[17]); } @@ -653,9 +599,8 @@ static void test_create_cache_by_name(gconstpointer user_data) { const char *requested; const char *expected_name; } names[] = { - {"lru", "LRU"}, {"LRU", "LRU"}, {"Lru", "LRU"}, {"fifo", "FIFO"}, - {"FIFO", "FIFO"}, {"fIfO", "FIFO"}, {"mru", "MRU"}, {"MRU", "MRU"}, - {"lruv0", "LRUv0"}, {"LRUv0", "LRUv0"}, + {"lru", "LRU"}, {"LRU", "LRU"}, {"Lru", "LRU"}, {"fifo", "FIFO"}, + {"FIFO", "FIFO"}, {"fIfO", "FIFO"}, {"mru", "MRU"}, {"MRU", "MRU"}, }; for (size_t i = 0; i < sizeof(names) / sizeof(names[0]); i++) { @@ -665,6 +610,9 @@ static void test_create_cache_by_name(gconstpointer user_data) { g_assert_cmpstr(cache->cache_name, ==, names[i].expected_name); cache->cache_free(cache); } + + // left out of the registry on purpose, see cacheAlgoRegistry.c + g_assert_null(create_cache_by_name("lruv0", cc_params, NULL)); } static void empty_test(gconstpointer user_data) { ; } @@ -711,10 +659,6 @@ int main(int argc, char *argv[]) { g_test_add_data_func("/libCacheSim/cacheAlgo_LIRS", reader, test_LIRS); g_test_add_data_func("/libCacheSim/cacheAlgo_LRU", reader, test_LRU); g_test_add_data_func("/libCacheSim/cacheAlgo_LRUv0", reader, test_LRUv0); - g_test_add_func("/libCacheSim/cacheAlgo_LRUv0_honors_admission", - test_LRUv0_honors_admission); - g_test_add_func("/libCacheSim/cacheAlgo_LRUv0_evicts_when_hit_grows_object", - test_LRUv0_evicts_when_hit_grows_object); g_test_add_data_func("/libCacheSim/cacheAlgo_LRU_K", reader, test_LRU_K); g_test_add_data_func("/libCacheSim/cacheAlgo_Merlin", reader, test_Merlin); g_test_add_data_func("/libCacheSim/cacheAlgo_MQ", reader, test_MQ);