Skip to content

Conversation

jianoaix
Copy link
Contributor

@jianoaix jianoaix commented Mar 12, 2025

Why are these changes needed?

TL;DR: The cache performance can be improved by 11x with circular queue

The existing feed cache uses a plain slice for cached data.
When it has to be updated with newly fetched data from DB, it may involve in significant amount of memory allocation and movement, e.g. in the existing code:

  • c.setCacheSegment(append(segment.data, data...), segment.start, end) for appending and
  • c.setCacheSegment(append(data, segment.data...), start, segment.end) for prepending.

This PR introduces a circular queue for caching the data and completely eliminates the need to re-allocate the memory for the cache (it always operate on pre-allocated queue).

Perf Benchmark

Benchmarked the cache extension backward (i.e. c.setCacheSegment(append(data, segment.data...), start, segment.end) case) for different cache sizes.

When there is 200k items in cache, performance improvement is 11x.

Note that 200k is a very practical number, for example, we cache for 2 days of batches, which will be 2 * 24 * 3600 = 172,800 batches in cache.

Before:

BenchmarkCacheExtendStart/CacheSize_10K-8        	   76166	     15203 ns/op	    6206 B/op	     133 allocs/op
BenchmarkCacheExtendStart/CacheSize_50K-8        	   73752	     15932 ns/op	    8422 B/op	     133 allocs/op
BenchmarkCacheExtendStart/CacheSize_200K-8       	   10000	    176257 ns/op	  274329 B/op	     133 allocs/op

After:

BenchmarkCacheExtendStart/CacheSize_10K-8        	   71349	     15441 ns/op	    6111 B/op	     133 allocs/op
BenchmarkCacheExtendStart/CacheSize_50K-8        	   77046	     15475 ns/op	    6111 B/op	     133 allocs/op
BenchmarkCacheExtendStart/CacheSize_200K-8       	   67117	     15726 ns/op	    6111 B/op	     133 allocs/op

Checks

  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@jianoaix jianoaix requested review from pschork, dmanc and ian-shim March 12, 2025 18:11
@jianoaix jianoaix merged commit 5a09ed7 into Layr-Labs:master Mar 12, 2025
10 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