From eaf14f3887497cd8a139b5a6350311aa91e29f7e Mon Sep 17 00:00:00 2001 From: windsornguyen Date: Sat, 26 Sep 2026 01:24:11 -0700 Subject: [PATCH] fix(S3FIFO): keep the 64-bit frequency when the main FIFO evicts S3FIFO, S3FIFOv0 and Clock2QPlus count an object's reads in an int64_t, and every hit increments it without bound until the main FIFO next passes the object. The main-FIFO eviction loop copies that count into an int before testing freq >= 1. An object read 2^31 times since its last pass narrows to a negative value, so the loop evicts the hottest object in the cache instead of reinserting it. The loops now copy the count into an int64_t. The new test gives the oldest main-FIFO object 2^31 reads and checks that the next eviction reinserts it and evicts the object behind it. Signed-off-by: Windsor Nguyen --- libCacheSim/cache/eviction/Clock2QPlus.c | 2 +- libCacheSim/cache/eviction/S3FIFO.c | 2 +- libCacheSim/cache/eviction/S3FIFOv0.c | 2 +- test/test_evictionAlgo.c | 43 ++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 3 deletions(-) diff --git a/libCacheSim/cache/eviction/Clock2QPlus.c b/libCacheSim/cache/eviction/Clock2QPlus.c index 373664fcf..3724587f0 100644 --- a/libCacheSim/cache/eviction/Clock2QPlus.c +++ b/libCacheSim/cache/eviction/Clock2QPlus.c @@ -367,7 +367,7 @@ static void Clock2QPlus_evict_main(cache_t *cache, const request_t *req) { while (!has_evicted && main->get_occupied_byte(main) > 0) { cache_obj_t *obj_to_evict = main->to_evict(main, req); DEBUG_ASSERT(obj_to_evict != NULL); - int freq = obj_to_evict->Clock2QPlus.freq; + int64_t freq = obj_to_evict->Clock2QPlus.freq; copy_cache_obj_to_request(params->req_local, obj_to_evict); if (freq >= 1) { // we need to evict first because the object to insert has the same obj_id diff --git a/libCacheSim/cache/eviction/S3FIFO.c b/libCacheSim/cache/eviction/S3FIFO.c index edb3bc935..8c226bfe7 100644 --- a/libCacheSim/cache/eviction/S3FIFO.c +++ b/libCacheSim/cache/eviction/S3FIFO.c @@ -377,7 +377,7 @@ static void S3FIFO_evict_main(cache_t *cache, const request_t *req) { while (!has_evicted && main_fifo->get_occupied_byte(main_fifo) > 0) { cache_obj_t *obj_to_evict = main_fifo->to_evict(main_fifo, req); DEBUG_ASSERT(obj_to_evict != NULL); - int freq = obj_to_evict->S3FIFO.freq; + int64_t freq = obj_to_evict->S3FIFO.freq; copy_cache_obj_to_request(params->req_local, obj_to_evict); if (freq >= 1) { // we need to evict first because the object to insert has the same obj_id diff --git a/libCacheSim/cache/eviction/S3FIFOv0.c b/libCacheSim/cache/eviction/S3FIFOv0.c index 2bd01b988..22c5cd3a2 100644 --- a/libCacheSim/cache/eviction/S3FIFOv0.c +++ b/libCacheSim/cache/eviction/S3FIFOv0.c @@ -374,7 +374,7 @@ static void S3FIFOv0_evict_main(cache_t *cache, const request_t *req) { while (!has_evicted && main_fifo->get_occupied_byte(main_fifo) > 0) { cache_obj_t *obj_to_evict = main_fifo->to_evict(main_fifo, req); DEBUG_ASSERT(obj_to_evict != NULL); - int freq = obj_to_evict->S3FIFO.freq; + int64_t freq = obj_to_evict->S3FIFO.freq; #if defined(TRACK_EVICTION_V_AGE) int64_t create_time = obj_to_evict->create_time; #endif diff --git a/test/test_evictionAlgo.c b/test/test_evictionAlgo.c index ba53c1f2a..2bbf93b1e 100644 --- a/test/test_evictionAlgo.c +++ b/test/test_evictionAlgo.c @@ -392,6 +392,47 @@ static void test_S3FIFO(gconstpointer user_data) { test_cache_algorithm(user_data, &test_data_truth[21]); } +// An object read more than INT32_MAX times since the main FIFO last passed it +// is the hottest object in the cache. Its 64-bit counter must survive the +// eviction loop's local copy, or the loop sees a negative count and evicts it. +static void test_S3FIFO_main_freq_beyond_int32(void) { + common_cache_params_t cc_params = {.cache_size = 100, .hashpower = 8}; + cache_t *cache = S3FIFO_init(cc_params, NULL); + request_t *req = new_request(); + req->obj_size = 1; + + // Objects 1 to 10 fill the small FIFO and 11 to 100 go straight to main, + // since nothing has been evicted yet. Two reads each promote 1 to 10. + for (obj_id_t id = 1; id <= 100; id++) { + req->obj_id = id; + cache->get(cache, req); + } + for (int round = 0; round < 2; round++) { + for (obj_id_t id = 1; id <= 10; id++) { + req->obj_id = id; + cache->get(cache, req); + } + } + + // Object 11 is the oldest in main. Give it 2^31 reads. + req->obj_id = 11; + cache_obj_t *hot = cache->find(cache, req, false); + g_assert_nonnull(hot); + hot->S3FIFO.freq = (int64_t)INT32_MAX + 1; + + // The next miss promotes 1 to 10, overfilling main, so main evicts from its + // head. Object 11 must be reinserted, and object 12 evicted in its place. + req->obj_id = 101; + cache->get(cache, req); + req->obj_id = 11; + g_assert_nonnull(cache->find(cache, req, false)); + req->obj_id = 12; + g_assert_null(cache->find(cache, req, false)); + + free_request(req); + cache->cache_free(cache); +} + static void test_S3FIFOv0(gconstpointer user_data) { test_cache_algorithm(user_data, &test_data_truth[22]); } @@ -474,6 +515,8 @@ int main(int argc, char *argv[]) { test_QDLP_FIFO); g_test_add_data_func("/libCacheSim/cacheAlgo_Random", reader, test_Random); g_test_add_data_func("/libCacheSim/cacheAlgo_S3FIFO", reader, test_S3FIFO); + g_test_add_func("/libCacheSim/cacheAlgo_S3FIFO_main_freq_beyond_int32", + test_S3FIFO_main_freq_beyond_int32); g_test_add_data_func("/libCacheSim/cacheAlgo_S3FIFOv0", reader, test_S3FIFOv0); #if defined(ENABLE_S4FIFO) && ENABLE_S4FIFO == 1