Copilot commented on code in PR #13893:
URL: https://github.com/apache/trafficserver/pull/13893#discussion_r4366151916
##########
src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc:
##########
@@ -398,3 +456,295 @@ TEST_CASE("CLFUS value metric is a floating-point
density", "[cache][ramcache][c
static_assert(v_hot > v_one, "CLFUS value metric does not increase with
hits");
static_assert(v_small > v_large, "CLFUS value metric does not decrease with
size");
}
+
+namespace
+{
+
+CryptoHash
+make_key(uint64_t id)
+{
+ CryptoHash key;
+ key.u32[0] = static_cast<uint32_t>(0xc0ffee00 + id);
+ key.u32[2] = static_cast<uint32_t>(0xdeadbeef + id);
+ key.u32[3] = static_cast<uint32_t>(0x12345678 + id);
+ return key;
+}
+
+} // namespace
+
+TEST_CASE("CLFUS decays hit count when sparing victims during eviction",
"[cache][ramcache][clfus]")
+{
+ CacheDisk disk;
+ init_disk(disk);
+ StripeSM stripe{&disk, 10, 0};
+ CacheVol cache_vol;
+ wire_stripe(stripe, cache_vol);
+
+ cache_config_ram_cache_compress = CACHE_COMPRESSION_NONE;
+ cache_config_ram_cache_use_seen_filter = 0;
+
+ RamCacheCLFUS rc;
+ rc.init(32 * 1024, &stripe);
+
+ auto buf = make_buffer(compressible_bytes(8 * 1024));
+ auto key1 = make_key(1);
+ auto key2 = make_key(2);
+ auto key3 = make_key(3);
+
+ REQUIRE(rc.put(&key1, buf.get(), 8 * 1024) == 1);
+ REQUIRE(rc.put(&key2, buf.get(), 8 * 1024) == 1);
+ REQUIRE(rc.put(&key3, buf.get(), 8 * 1024) == 1);
+
+ RamCacheCLFUSEntry *e1 = RamCacheCLFUSTestAccess::find_entry(rc, key1);
+ REQUIRE(e1 != nullptr);
+ e1->hits = 16; // Give key1 high hits so it exceeds average value
+
+ // key4 triggers an eviction pass
+ auto key4 = make_key(4);
+ rc.put(&key4, buf.get(), 8 * 1024);
+
+ // e1 was spared on its first pass as a victim (16 -> 8), then taken on the
+ // second pass once the requeue budget ran out and put back by
+ // _requeue_victims() (8 -> 7). Without the spared-victim decay it would be
15.
+ CHECK(e1->hits == 7);
+}
+
+TEST_CASE("CLFUS unconditionally promotes resident entries to the tail on
hits", "[cache][ramcache][clfus]")
+{
+ CacheDisk disk;
+ init_disk(disk);
+ StripeSM stripe{&disk, 10, 0};
+ CacheVol cache_vol;
+ wire_stripe(stripe, cache_vol);
+
+ cache_config_ram_cache_compress = CACHE_COMPRESSION_NONE;
+ cache_config_ram_cache_use_seen_filter = 0;
+
+ RamCacheCLFUS rc;
+ rc.init(64 * 1024, &stripe);
+
+ auto buf = make_buffer(compressible_bytes(4 * 1024));
+ auto key1 = make_key(1);
+ auto key2 = make_key(2);
+ auto key3 = make_key(3);
+
+ REQUIRE(rc.put(&key1, buf.get(), 4 * 1024) == 1);
+ REQUIRE(rc.put(&key2, buf.get(), 4 * 1024) == 1);
+ REQUIRE(rc.put(&key3, buf.get(), 4 * 1024) == 1);
+
+ RamCacheCLFUSEntry *e1 = RamCacheCLFUSTestAccess::find_entry(rc, key1);
+ REQUIRE(e1 != nullptr);
+ // Initial order in _lru[0] has e1 at the head
+ CHECK(RamCacheCLFUSTestAccess::lru_head(rc) == e1);
+
+ // Inflate _average_value so e1's value is lower than average
+ RamCacheCLFUSTestAccess::set_average_value(rc, 1000.0);
+
+ // get() on key1 must unconditionally promote e1 to the tail of _lru[0]
+ Ptr<IOBufferData> got;
+ CHECK(rc.get(&key1, &got) == 1);
+ CHECK(RamCacheCLFUSTestAccess::lru_tail(rc) == e1);
+ CHECK(RamCacheCLFUSTestAccess::lru_head(rc) != e1);
+}
+
+TEST_CASE("CLFUS history entry is refreshed on get() hit",
"[cache][ramcache][clfus]")
+{
+ CacheDisk disk;
+ init_disk(disk);
+ StripeSM stripe{&disk, 10, 0};
+ CacheVol cache_vol;
+ wire_stripe(stripe, cache_vol);
+
+ cache_config_ram_cache_compress = CACHE_COMPRESSION_NONE;
+ cache_config_ram_cache_use_seen_filter = 0;
+
+ RamCacheCLFUS rc;
+ rc.init(12 * 1024, &stripe);
+
+ auto buf = make_buffer(compressible_bytes(8 * 1024));
+ auto key1 = make_key(1);
+ auto key2 = make_key(2);
+
+ REQUIRE(rc.put(&key1, buf.get(), 8 * 1024) == 1);
+ // key2 cannot fit in 12KB (8KB + 8KB > 12KB), routed to history (_lru[1])
+ CHECK(rc.put(&key2, buf.get(), 8 * 1024) == 0);
+
+ RamCacheCLFUSEntry *e2 = RamCacheCLFUSTestAccess::find_entry(rc, key2);
+ REQUIRE(e2 != nullptr);
+ REQUIRE(e2->flag_bits.lru == 1); // in history
+
+ uint64_t hits_before = e2->hits;
+ Ptr<IOBufferData> got;
Review Comment:
The history-recency assertion cannot detect a regression: `e2` is the only
history entry, so it is already the tail before `get()`, and the check still
passes if the new remove/enqueue operations are deleted. Add another history
entry and require `e2` not to be the tail before the lookup, then verify that
the lookup moves it there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
