Skip to content

perf: add synchronous(Normal) + optimize_on_close to record store SQLite - #3577

Merged
ellie merged 2 commits into
atuinsh:mainfrom
markovejnovic:marko/bench-record-store-sqlite-opts
Jul 7, 2026
Merged

perf: add synchronous(Normal) + optimize_on_close to record store SQLite#3577
ellie merged 2 commits into
atuinsh:mainfrom
markovejnovic:marko/bench-record-store-sqlite-opts

Conversation

@markovejnovic

@markovejnovic markovejnovic commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

This PR adds two commits:

  • The first commit adds benchmarking support to crates/atuin-client.
  • The second commit adds a tiny performance optimization to record/sqlite_store.rs which requests sqlite pragma optimize on close as well as prefers the WAL rather than immediate fsync.

Hand-rolled PR (I hated claude's output).

Benchmarking

I used divan since this was the existing precedent. To run benchmarks:

cargo bench -p atuin-client

I think this is a little bit naive, and ideally we'd have rich benchmarking in CI, but I think that could be considered a future improvement.

If you think it's worth dropping the benchmarking addition from this PR (in lieu of a future, better, CI-integrated addition), I'm totally down to do that =)

Had the directory structure mirror the source tree

Optimization

There is a semantic change that occurred here -- in case the user's machine loses power, we risk losing the WAL on their machine, but:

  • for the remote case, it's fine, since we'll pick up on those changes in the next sync with the remote.
  • the local case does risk losing data, but this seems to be in-line with the history's database.

Results

Tested on an M4 mac.

At 171f7643d4337165ab96d15c010fb6638a3dfcb9 (before):

record_store         fastest       │ slowest       │ median        │ mean          │ samples │ iters
╰─ record                          │               │               │               │         │
   ╰─ sqlite_store                 │               │               │               │         │
      ╰─ push_batch                │               │               │               │         │
         ├─ 1        138.6 µs      │ 5.801 ms      │ 247.8 µs      │ 280.1 µs      │ 2504    │ 2504
         ├─ 10       177.7 µs      │ 5.204 ms      │ 200.4 µs      │ 215.3 µs      │ 3842    │ 3842
         ╰─ 100      617.7 µs      │ 8.771 ms      │ 718.1 µs      │ 779 µs        │ 2430    │ 2430

At 4f30c909f459676857e564b338cfd2622794714f (after):

record_store         fastest       │ slowest       │ median        │ mean          │ samples │ iters
╰─ record                          │               │               │               │         │
   ╰─ sqlite_store                 │               │               │               │         │
      ╰─ push_batch                │               │               │               │         │
         ├─ 1        98.83 µs      │ 5.41 ms       │ 183.7 µs      │ 189.3 µs      │ 4770    │ 4770
         ├─ 10       151.6 µs      │ 6.139 ms      │ 170.2 µs      │ 186 µs        │ 4736    │ 4736
         ╰─ 100      578.1 µs      │ 8.483 ms      │ 661 µs        │ 704.7 µs      │ 2890    │ 2890

I had claude generate a comparison table:

  ┌────────────────┬─────────────────┬────────────────┬────────┐
  │   Benchmark    │ Before (median) │ After (median) │ Change │
  ├────────────────┼─────────────────┼────────────────┼────────┤
  │ push_batch/1   │ 247.8 µs        │ 183.7 µs       │ -26%   │
  ├────────────────┼─────────────────┼────────────────┼────────┤
  │ push_batch/10  │ 200.4 µs        │ 170.2 µs       │ -15%   │
  ├────────────────┼─────────────────┼────────────────┼────────┤
  │ push_batch/100 │ 718.1 µs        │ 661.0 µs       │ -8%    │
  └────────────────┴─────────────────┴────────────────┴────────┘

Worth noting that my machine appears to be very noisy. I think less noisy benchmarks require we have a hosted bare-metal machine for this, specifically designed for benchmarking, but I think this is sufficient proof that the flags I've added are valuable.

Checks

  • I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
  • I have checked that there are no existing pull requests for the same thing

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Fossier: Manual Review Requested

@markovejnovic is a new contributor. A maintainer should review this PR before merging.

Score Breakdown

Total Score: 52.6/100 | Confidence: 100% | Outcome: REVIEW

Signal Value Score Weight
account_age 3914 1.00 0.09
public_repos 85 1.00 0.05
contribution_history 96 0.48 0.05
follower_ratio 1.39 0.70 0.05
bot_signals False 0.50 0.04
open_prs_elsewhere 37 0.00 0.09
closed_prs_elsewhere 49 0.00 0.10
merged_prs_elsewhere 298 1.00 0.08
prior_interaction 0 0.00 0.08
activity_velocity 4 0.33 0.08
pr_content ... 1.00 0.08
commit_email marko@harmont.dev 0.80 0.04
pr_description ... 0.80 0.05
repo_stars 30469 0.30 0.04
org_membership 0 0.20 0.03
commit_verification ... 0.65 0.04
contributor_stars 25 0.50 0.04

@markovejnovic
markovejnovic force-pushed the marko/bench-record-store-sqlite-opts branch 2 times, most recently from dbe3e2a to c46b2f2 Compare July 7, 2026 22:41
@markovejnovic
markovejnovic marked this pull request as ready for review July 7, 2026 22:46
@markovejnovic
markovejnovic marked this pull request as draft July 7, 2026 22:47
@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR brings record/sqlite_store.rs in line with the existing history database (database.rs) by adding optimize_on_close and synchronous=Normal — both already used there — and adds a divan benchmark suite for push_batch latency.

  • The SQLite change is a safe, well-precedented two-liner: WAL + synchronous=Normal is explicitly recommended by SQLite docs, and optimize_on_close is a no-op on a cold database.
  • The benchmark wires up divan correctly (dev-dependency, harness = false, temp-dir lifecycle managed via _temp_dir), but each iteration creates and tears down a fresh database, so it measures cold-start write performance rather than steady-state.

Confidence Score: 5/5

Safe to merge — the SQLite tweak mirrors an identical configuration already in production use for the history database.

Both changes are low-risk: the pragma settings are already proven in database.rs, and the benchmark only affects cargo bench, not production paths.

No files require special attention.

Important Files Changed

Filename Overview
crates/atuin-client/src/record/sqlite_store.rs Adds optimize_on_close and synchronous=Normal — matches the existing history DB configuration in database.rs
crates/atuin-client/benches/record/sqlite_store.rs New divan benchmark for push_batch; db is dropped inside bench_values timing window, so pool close (including optimize_on_close) is included in the measured time
crates/atuin-client/Cargo.toml Correctly adds divan and tempfile to dev-dependencies and declares the bench harness
crates/atuin-client/benches/record_store.rs Minimal divan entrypoint, correct
crates/atuin-client/benches/record/mod.rs Module re-export, no issues

Reviews (2): Last reviewed commit: "perf: add synchronous(Normal) + optimize..." | Re-trigger Greptile

@markovejnovic
markovejnovic force-pushed the marko/bench-record-store-sqlite-opts branch 3 times, most recently from eb12af9 to 0037ccd Compare July 7, 2026 23:12
Add push_batch (1/10/100) benchmarks using divan with realistic payload
sizes (~300 bytes data, ~150 bytes CEK). Setup (DB creation, migrations,
record generation) is excluded from timed regions via Bencher API.
Match the pattern already used by the history database, KV store, scripts
database, and AI store. With WAL mode, synchronous=Normal skips fsync on
most commits for significantly faster writes.
@markovejnovic
markovejnovic force-pushed the marko/bench-record-store-sqlite-opts branch from 0037ccd to 4f30c90 Compare July 7, 2026 23:13
@markovejnovic
markovejnovic marked this pull request as ready for review July 7, 2026 23:24

@ellie ellie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, thank you!

I think definitely a nice starting place for benchmarks for us, but in the long term we need a stable CI run so that we can track things properly - as you suggested!

@ellie
ellie merged commit 4b46200 into atuinsh:main Jul 7, 2026
26 checks passed
ellie pushed a commit that referenced this pull request Jul 8, 2026
This PR adds two commits:

- The first commit adds another benchmark to `crates/atuin-client`.
- The second commit adds a tiny performance optimization to
`reorder_fuzzy` which avoids an unnecessary clone.

Hand-rolled PR (I hated claude's output).

## Benchmarking

I did a couple more changes to benchmarking.

- In #3577, I naively used `rand`,
but I realized this is pretty bad for benchmark reproducibility, so I
created a new `BenchCtx` utility which is a more stable random number
generator (and a dumping ground for similar things where we want
stability, another example in this PR being the "now" timestamp).
- I reduced the minimum time per iteration of a benchmark to be `1s`
from `5s`.


## Optimization

Pretty bog-standard removal of a potentially large clone.
  
### Results

Tested on an M4 mac.

#### Before

```
benchmarks                 fastest       │ slowest       │ median        │ mean          │ samples │ iters
├─ ordering                              │               │               │               │         │
│  ╰─ reorder_fuzzy_bench                │               │               │               │         │
│     ├─ 10                2.124 µs      │ 31.91 µs      │ 2.499 µs      │ 2.593 µs      │ 47246   │ 47246
│     ├─ 200               43.62 µs      │ 81.62 µs      │ 49.2 µs       │ 49.58 µs      │ 2488    │ 2488
│     ╰─ 1000              230.2 µs      │ 305.4 µs      │ 252.4 µs      │ 256.5 µs      │ 495     │ 495
╰─ record                                │               │               │               │         │
   ╰─ sqlite_store                       │               │               │               │         │
      ╰─ push_batch                      │               │               │               │         │
         ├─ 1              99.45 µs      │ 1.326 ms      │ 187.6 µs      │ 189.9 µs      │ 945     │ 945
         ├─ 10             155.5 µs      │ 5.674 ms      │ 173.4 µs      │ 191.3 µs      │ 957     │ 957
         ╰─ 100            582.2 µs      │ 6.717 ms      │ 659.4 µs      │ 697.9 µs      │ 568     │ 568
```

#### After

```
benchmarks                 fastest       │ slowest       │ median        │ mean          │ samples │ iters
├─ ordering                              │               │               │               │         │
│  ╰─ reorder_fuzzy_bench                │               │               │               │         │
│     ├─ 10                1.249 µs      │ 30.54 µs      │ 1.582 µs      │ 1.602 µs      │ 49473   │ 49473
│     ├─ 200               25.62 µs      │ 59.7 µs       │ 29.16 µs      │ 29.55 µs      │ 2616    │ 2616
│     ╰─ 1000              135.4 µs      │ 190.3 µs      │ 152.9 µs      │ 155.6 µs      │ 519     │ 519
╰─ record                                │               │               │               │         │
   ╰─ sqlite_store                       │               │               │               │         │
      ╰─ push_batch                      │               │               │               │         │
         ├─ 1              97.95 µs      │ 1.344 ms      │ 178.8 µs      │ 179.3 µs      │ 970     │ 970
         ├─ 10             155.9 µs      │ 4.397 ms      │ 171.3 µs      │ 181.5 µs      │ 994     │ 994
         ╰─ 100            578.9 µs      │ 8.866 ms      │ 653.4 µs      │ 711.1 µs      │ 565     │ 565
```

#### Comparison

```
  ┌────────────────────┬─────────────────┬────────────────┬────────────────────┐
  │     Benchmark      │ Before (median) │ After (median) │      Speedup       │
  ├────────────────────┼─────────────────┼────────────────┼────────────────────┤
  │ reorder_fuzzy 10   │ 2.499 µs        │ 1.582 µs       │ 1.58× (37% faster) │
  ├────────────────────┼─────────────────┼────────────────┼────────────────────┤
  │ reorder_fuzzy 200  │ 49.20 µs        │ 29.16 µs       │ 1.69× (41% faster) │
  ├────────────────────┼─────────────────┼────────────────┼────────────────────┤
  │ reorder_fuzzy 1000 │ 252.4 µs        │ 152.9 µs       │ 1.65× (39% faster) │
  └────────────────────┼─────────────────┼────────────────┼────────────────────┘
```

## Checks

- [x] I am happy for maintainers to push small adjustments to this PR,
to speed up the review cycle
- [x] I have checked that there are no existing pull requests for the
same thing
taylordotfish pushed a commit that referenced this pull request Jul 9, 2026
### Bug Fixes

- *(ai)* Dispatch skills registered in the slash command registry
([#3593](#3593))
- *(ci)* Fossier install in scan workflow
([#3485](#3485))
- *(i18n)* Fix typos in Russian localization
([#3575](#3575))
- *(nu)* Use `char -u 1b` for ESC in OSC 133 sequences
([#3530](#3530))
- *(nu)* Suppress error when `ATUIN_HISTORY_ID` is missing in
`pre_prompt` hook
([#3587](#3587))
- *(pi)* Observe tool events instead of registering a bash tool
([#3557](#3557))
- *(pty-proxy)* Set `$SHELL` to the spawned shell
([#3548](#3548))
- *(search)* Fix terminal clearing with latest Ratatui
([#3578](#3578))
- *(sync)* Skip records that fail to decrypt or decode instead of
failing the whole store
([#3569](#3569))
- Atuin hangs when attempting to spawn daemon from Ctrl+R invocation
([#3502](#3502))
- Capture session ID from stream headers rather than final event
([#3531](#3531))
- Doctor resiliency fo runknown platforms + openbsd warning
([#3551](#3551))
- Double input on arrow keys in AI setup prompt on Windows
([#3552](#3552))
- Exclude AI agent commands from zsh-autosuggestions
([#3567](#3567))
- Silence shellcheck SC2016 on literal `$all-user` author filter
- Respect `store_failed` when using daemon
([#3571](#3571))


### Documentation

- Highlight `Ctrl-r` keybinding on docs page
([#3489](#3489))
- Document store purge workflow
([#3544](#3544))
- Fix command example typo in documentation
([#3536](#3536))
- Make commented-out lines in `config.toml` match real defaults
([#3583](#3583))
- Add fish shell cleanup step to uninstall instructions
([#3582](#3582))


### Features

- *(doctor)* Add whether daemon is enabled to `doctor` output
([#3572](#3572))
- *(pty-proxy)* Add `--shell` flag to override the spawned shell
([#3327](#3327))
- Setup fossier to stop bot slop prs
([#3482](#3482))
- Capture command output + expose to new `atuin_output` tool
([#3510](#3510))
- Cache user contexts on load until `/reload`
([#3525](#3525))
- Create database integration tests for atuin-server
([#3514](#3514))
- Add `/model` slash command for changing models
([#3576](#3576))
- Add mcp server for history tools and expand search filters
([#3581](#3581))
- Add status bar with model and usage information
([#3591](#3591))


### Miscellaneous Tasks

- *(rustdoc)* Fix Rustdoc warnings
([#3585](#3585))
- *(warnings)* Fix compile warnings with latest dependencies
([#3586](#3586))
- Vouch for all existing contributors
([#3486](#3486))
- Update GitHub app token format
- Update to Rust 1.96.1
([#3568](#3568))
- Adopt `derive_more` to reduce boilerplate across the codebase
([#3573](#3573))


### Performance

- *(search)* Scan history by recency until N unique
([#3553](#3553))
- Add `synchronous(Normal)` + `optimize_on_close` to record store SQLite
([#3577](#3577))
- Remove unnecessary clones in a hot path
([#3580](#3580))


### Refactor

- Implement `From<sqlx::Error>` and clean up `fix_error`
([#3484](#3484))
- Pull `fn into_utc` into `atuin-server-database` crate
([#3487](#3487))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants