Conversation
rittneje
reviewed
Apr 8, 2026
stmtCacheSize is immutable after connection open, so checking it before the lock avoids mutex overhead when cache is not enabled.
When stmtCacheSize <= 0, stmtCacheCount >= stmtCacheSize is always true, so the explicit check is unnecessary.
Finalize all cached statements even if one fails. Leaving a finalized statement in the cache map would be a use-after-finalize bug per SQLite documentation.
prepareWithCache now delegates to prepare and sets cacheKey afterward, removing the useCache boolean parameter.
This avoids an unnecessary reset when the cache is full, guarantees a statement cannot enter the cache without being reset/cleared, and fixes a leak where sqlite3_finalize was not called when reset failed.
Clarify that each connection in the sql.DB pool maintains its own independent statement cache.
Owner
Author
|
@rittneje Thank you |
Collaborator
|
@mattn Reflecting on this more, it seems like this approach doesn't quite account for evicting stuff from the cache. For example, let's say the cache size is 5. My first 5 queries might be one-time only, and they will all be cached forever. And then the 6th query - which maybe gets run frequently - never gets cached. |
Owner
Author
|
@rittneje Right. I'll implement LRU cache. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change adds an opt-in connection-local statement cache to
go-sqlite3, enabled via the DSN parameter_stmt_cache_size=N.The motivation is that, in this benchmark, the main gap in
mattn/go-sqlite3was not cgo alone but repeatedprepare/finalizeon hotQueryRowpaths. Reusing prepared statements at the driver level closes most of that gap without requiring application code to switch to explicitdb.Prepare(...).Implementation details:
_stmt_cache_sizeDSN parsingQuery/Execpaths_stmt_cache_sizeis setBenchmark workload:
SELECT id, name, hp, attack FROM monster WHERE id = ?P1: about 16 goroutinesP10: about 160 goroutinesP100: about 1600 goroutinesBenchmark commands used:
For
mattn/go-sqlite3:For
modernc:Benchmark results on
2026-04-07(Ryzen 7 7735HS,WSL2):In this workload, the cached version improves
mattnsubstantially:With the cache enabled,
mattnis fastest in 3 of 4 measured cases here (Seq,P10,P100), whilemoderncremains slightly faster inP1.This is intentionally opt-in because statement caching changes connection behavior and should be evaluated conservatively in real applications.