evict least-recently-used stmt when cache is full#1388
Conversation
mattn
force-pushed
the
stmt-cache-lru
branch
2 times, most recently
from
a81a7ee to
c70622e
Compare
| C.sqlite3_finalize(victim.s) | ||
| victim.s = nil | ||
| } | ||
| victim.c = nil |
There was a problem hiding this comment.
I'm a little confused why we are doing this. Isn't it falling out of scope?
| c.stmtCacheCount-- | ||
| c.stmtCacheBuf[c.stmtCacheCount] = nil | ||
| s.closed = false | ||
| s.cls = false |
There was a problem hiding this comment.
Why are we setting this to false?
Likewise, why are we setting s.t to the empty string?
There was a problem hiding this comment.
extracted a finalizeCachedStmt helper shared by the eviction path and closeCachedStmtsLocked; dropped the redundant s.t = "" reset in takeCachedStmt (cached stmts always have t == "") and left a comment explaining why closed/cls still need to be reset.
|
|
||
| func (c *SQLiteConn) putCachedStmt(s *SQLiteStmt) bool { | ||
| if c == nil || s == nil || s.s == nil || s.cacheKey == "" { | ||
| if c == nil || s == nil || s.s == nil || s.cacheKey == "" || c.stmtCacheSize <= 0 { |
There was a problem hiding this comment.
How would s.cacheKey be set if there is no stmt cache?
There was a problem hiding this comment.
prepareWithCache now only sets cacheKey when the cache is enabled, so the redundant size check in putCachedStmt is gone.
2 participants
Follow-up to #1387, addressing rittneje's comment: #1387 (comment)
When the cache was full,
putCachedStmtrejected the new entry instead of evicting anything. This meant the first N cached statements squatted on every slot forever, and a hot query prepared later would never benefit from caching. Now we evict the least-recently-used entry to make room.The cache is a single preallocated
[]*SQLiteStmtof length_stmt_cache_size, ordered LRU-first (buf[0]is next to be evicted,buf[count-1]is the most recently put). Put at the tail is O(1) when not full; eviction shifts the remaining entries left by one. Take does a backward linear scan (MRU-end first) and shifts the right side left. For the small cache sizes users typically configure (a handful up to a few dozen), these O(N) operations are cache-friendly pointer moves and outperform the previous map + linked list design on every hit-path microbenchmark.No per-operation allocation, no map, no linked list, no extra fields on
SQLiteStmt.Benchmarks
BenchmarkStmtCacheis added in this PR. Measured on linux/amd64, AMD Ryzen 7 7735HS,benchstatover 10 runs.vs master (before this PR)
Interpretation
Hit path (working set fits in cache). All three hit cases come out faster than master or within noise, with one fewer allocation per operation. Replacing the previous
map[string][]*SQLiteStmt+ linked list design with a flat preallocated slice removes the map lookup, the per-put slice reallocation, and all list pointer bookkeeping.Evict path. The
_evictcases cycle through N distinct queries with N > cache size. This is the worst case for LRU under uniform round-robin access:Master's 50% hit rate is not the result of a smart policy — it comes from the "freeze the cache once full" behavior accidentally suiting a uniform cyclic pattern. On realistic workloads (some hot queries, some cold, not all arriving at startup) the old policy leaves hot queries permanently uncached, which is exactly the problem rittneje raised. The +40-48% here reflects LRU correctly evicting old entries; it is not a regression in the fix.
Users whose real working set is larger than
_stmt_cache_sizeshould increase the cache size or disable the cache (_stmt_cache_size=0, the default).Impact on ad-hoc
db.QueryRowAdditional numbers from lirlia/go-sqlite-performance —
SELECT id, name, hp, attack FROM monster WHERE id = ?against a 1000-row table, linux/amd64, AMD Ryzen 7 7735HS, single connection, 5 runs. Compares repeateddb.QueryRow(query, args)with and without_stmt_cache_size=32:On this realistic hot-path workload the cache (introduced in #1387 and correctly LRU-managed in this PR) cuts per-query cost by roughly 37-39% under parallelism and 26% serially. The
BenchmarkStmtCachemicrobenchmark above isolates the cache data-structure change; this table shows what it means end-to-end for a user callingdb.QueryRowin a loop.