fix: batch WAL frame writes into one write_at per commit - #641
Merged
Conversation
WalWriter::append_frame issued its own write_at syscall per dirty page instead of batching a transaction's frames into a single write. For a commit touching N scattered pages that's N separate write syscalls in the hot commit path before the one fsync in sync(). Accumulate frames in a pending buffer during append_frame and issue one write_at covering the whole run in sync(), which still fsyncs exactly once per commit (ADR-0026 unchanged — only the writes feeding that fsync are batched). Profiling update_batch_tx_wal (#635) shows this WAL-layer inefficiency was real but not the dominant cost of the benchmark's 7x gap vs C SQLite: the commit path (open_existing rescan + frame writes + fsync) is ~6-11ms of the ~27-30ms total, with the rest attributable to VDBE table-scan interpretation overhead for the unindexed WHERE clause (~1us/row over 16,700 rows) — filed as follow-up tickets rather than chased in this one. spend: within estimate (single profiling-driven fix, no scope creep)
iheitlager
force-pushed
the
fix/635-wal-batch-frame-writes
branch
from
August 29, 2026 08:59
f9f4f21 to
bb63744
Compare
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.
Summary
WalWriter::append_frameissued its ownwrite_atsyscall per dirty page instead of batching a transaction's frames into a single write — an O(n) syscall pattern in the hot commit path.pendingbuffer duringappend_frame;sync()now issues onewrite_atcovering the whole run, then fsyncs exactly once (ADR-0026's per-commit rescan behavior is unchanged — only the writes feeding that one fsync are batched).Profiling (#635)
update_batch_tx_walbenchmark: sqlite-rs 27-32ms vs C SQLite oracle ~4.3ms onbench_1mb.db.Instrumented the commit path directly (not via a spawned process, to avoid process-startup noise): for this benchmark's 34-page commit,
open_existingrescan is ~6-7ms and the batched write+fsync is ~4.5-5ms — roughly ~11ms of the ~27-32ms total. The remaining ~15-20ms is VDBE table-scan interpretation overhead for the unindexedWHERE bucket = 5clause (~1µs/row × 16,700 rows) — not a WAL-layer cost, and not something this PR attempts to fix.This fix is a real, tested improvement (removes N syscalls → 1 for multi-page commits) but does not close the 7x gap for this specific benchmark on its own. Filed follow-ups for the remaining costs:
spend: within estimate (single profiling-driven fix, no scope creep)
Closes #635
Test plan
cargo test --lib— 953 passedcargo test --test tier0 --test tier1 --test tier2 --test tier3— all pass, includingt3_wal_writing_and_live_interopcargo clippy --lib -- -D warnings— cleanupdate_batch_tx_waltimings, batching alone doesn't move this particular benchmark much (page count too small — 34 pages — for syscall count to dominate), consistent with the profiling above