Skip to content

fix(S3FIFO): keep the 64-bit frequency when the main FIFO evicts - #342

Merged
1a1a11a merged 1 commit into
1a1a11a:developfrom
windsornguyen:fix/s3fifo-main-freq-narrowing
Sep 26, 2026
Merged

1a1a11a merged 1 commit into
1a1a11a:developfrom
windsornguyen:fix/s3fifo-main-freq-narrowing

Conversation

@windsornguyen

Copy link
Copy Markdown
Contributor

What does this PR do?

S3FIFO, S3FIFOv0 and Clock2QPlus keep an object's read count in an int64_t (cacheObj.h), and every hit increments it without bound until the main FIFO passes the object again. The main-FIFO eviction loop copies the count into an int before testing freq >= 1:

int freq = obj_to_evict->S3FIFO.freq;

An object read 2^31 times since its last pass narrows to a negative value, so the loop takes the evict branch and removes the hottest object in the cache instead of reinserting it. The fix copies the count into an int64_t at all three sites. S4FIFO and SFIFO store an int32_t, so they do not narrow.

Type of change

  • Bug fix

How was it tested?

The new /libCacheSim/cacheAlgo_S3FIFO_main_freq_beyond_int32 test fills a 100-object cache, promotes objects 1 to 10, and gives object 11, the oldest in the main FIFO, 2^31 reads. The next miss makes the main FIFO evict from its head. Object 11 must be reinserted and object 12 evicted.

Build Result
develop + the test fails: object 11 is evicted
this PR passes: object 11 is kept with count 2, object 12 is evicted

Release build in ubuntu:24.04 with Ninja:

  • ctest --test-dir _build --output-on-failure passes (10/10)
  • Build is warning-free

Checklist

  • Added or updated tests

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 <win@dedaluslabs.ai>
@1a1a11a

1a1a11a commented Sep 26, 2026

Copy link
Copy Markdown
Owner

Thank you!

@1a1a11a
1a1a11a merged commit 7c1a72b into 1a1a11a:develop Sep 26, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants