Skip to content

perf(state): config-gated SQLite PRAGMA tuning for large DBs (salvage #71755) - #77630

Merged
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/71755-pragma-tuning
Aug 3, 2026
Merged

perf(state): config-gated SQLite PRAGMA tuning for large DBs (salvage #71755)#77630
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/71755-pragma-tuning

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Context

Users with large state.dbs (hundreds of MB — heavy browser/tool sessions) can only tune SQLite via wal_autocheckpoint/journal_size_limit today. This adds three config-gated, non-durability pragmas — database.cache_size, database.mmap_size, database.temp_store — and applies the pragma pass to the read-only connection and the per-thread WAL readers (cache_size/mmap_size are per-connection, so readers never benefited before). WHO benefits: opt-in only — users who set these keys in config.yaml, i.e. large-DB power users; a default install is byte-for-byte unchanged.

Measured impact

211 MB synthetic DB (360k rows), heavy read scan, default page cache vs cache_size=-262144 + 256 MB mmap, median of 5:

scenario before after delta
cold-ish full scan 0.484 s 0.468 s +3.3%
warm re-scan 0.211 s 0.163 s +22.9%

Honest caveat: config-gated and default-off; the win only exists for users who opt in on a large DB, and is workload-dependent (warm repeated reads benefit most; cold I/O-bound scans barely move).

Safety audit (the #64091 lesson)

No durability pragmas: the allowlisted tuple gains only cache_size, mmap_size, temp_store — no synchronous, journal_mode, locking_mode, or fullfsync. Values apply ONLY when present in config.yaml; integer coercion warns-and-skips garbage; apply_database_pragmas remains best-effort try/except so a failing pragma cannot break open. The docstring's journal-mode-ownership warning stays accurate. Per-reader cost is bounded: the reader connection is thread-local, so the pragma pass runs once per thread lifetime, and the config read is the mtime-cached fast path.

Provenance

Salvage of #71755 by @crayfish-ai (both commits authorship-preserved). Follow-up commit adds the missing guard test: config-set pragmas must reach the writer, read-only, and per-thread reader connections (with a discriminating -16000 vs SQLite's -2000 default so a revert can't accidentally pass), and a no-config run must leave defaults untouched. Mutation-checked: reverting the pragma wiring turns the test RED.

Verification

  • 165 passed (tests/test_hermes_state.py, -p no:randomly) + the new end-to-end pragma test.
  • Simplify pass: no material findings (coercion loop reused, not duplicated; per-thread cost traced to once-per-thread-lifetime).

Closes #71755.

@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) August 3, 2026 11:59
@alt-glitch alt-glitch added type/perf Performance improvement or optimization P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state needs-decision Awaiting maintainer decision before any implementation labels Aug 3, 2026
@kshitijk4poor
kshitijk4poor force-pushed the salvage/71755-pragma-tuning branch from 38fd2f7 to 75b18e9 Compare August 3, 2026 13:13
@Ruanjq98

Ruanjq98 commented Aug 3, 2026

Copy link
Copy Markdown

Code Review: #77630

Verdict: Approve

DB PRAGMAs: well-documented addition of cache_size, mmap_size, temp_store. Safe best-effort.

LGTM - Reviewed diff. Changes are sound.

crayfish-ai and others added 3 commits August 3, 2026 20:07
…agmas

Addresses review from @teknium1 on PR NousResearch#71755:

- Extended apply_database_pragmas() to handle cache_size, mmap_size,
  and temp_store from config.yaml (alongside existing wal_autocheckpoint
  and journal_size_limit). No hardcoded defaults — all values are
  opt-in via config.yaml, avoiding policy conflicts with other PRs.
- Applied to ALL connection types: writer (_connect_and_init),
  read_only cross-profile attach, and WAL per-thread readers
  (_get_read_conn). Previously PRAGMAs only ran on the writer path.
- Removed inline PRAGMAs from _connect_and_init — single source of
  truth in apply_database_pragmas().
- Documented config keys with examples in function docstring.
…ection types

E2E guard for the salvaged PR NousResearch#71755: database.cache_size/mmap_size/
temp_store from config.yaml must reach the writer connection, the
read-only cross-profile attach, and the WAL per-thread reader — and a
default install (no database: keys) must keep byte-identical SQLite
defaults on every connection type. Also covers integer-coercion
rejection of garbage values for the three new keys.

cache_size uses -16000 (not the doc example -2000) because -2000 is
SQLite's compiled-in default and would not discriminate a regression.
@kshitijk4poor
kshitijk4poor force-pushed the salvage/71755-pragma-tuning branch from 75b18e9 to dff7eb0 Compare August 3, 2026 14:38
@kshitijk4poor
kshitijk4poor merged commit 7026177 into NousResearch:main Aug 3, 2026
38 checks passed
@kshitijk4poor
kshitijk4poor deleted the salvage/71755-pragma-tuning branch August 5, 2026 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants