diff --git a/CHANGELOG.md b/CHANGELOG.md index c8d65cb6..5f47fb81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ All notable changes to sqlite-rs. Format follows [Keep a Changelog](https://keep ### Fixed +- `WalWriter::append_frame` issued its own `write_at` syscall per dirty + page instead of batching a transaction's frames into a single write — + an O(n) syscall pattern in the commit hot path. Frames now accumulate + in a pending buffer and `sync()` issues one `write_at` covering the + whole run, still fsyncing exactly once per commit (ADR-0026's + per-commit rescan behavior unchanged — only the writes feeding that + fsync are batched). Filed as follow-ups rather than chased further + here: #639 (VDBE table-scan interpretation overhead) and #640 + (ADR-0026's flagged per-commit WAL rescan cost) (#635). + - Implicit-whole-table-group aggregates (`count(*)`/`sum`/etc. with a `WHERE` clause, no `GROUP BY`) routed through `compile_grouped_scan`, which unconditionally opened a `Sorter` even though there is no diff --git a/src/pager/checkpoint.rs b/src/pager/checkpoint.rs index 7e5c8dd9..cecdaeff 100644 --- a/src/pager/checkpoint.rs +++ b/src/pager/checkpoint.rs @@ -345,7 +345,7 @@ mod tests { let (vfs, db_path) = setup(512); let wal_path = companion_path(&db_path, "-wal"); let header = WalHeader::new(true, 512, 0x7777, 0x8888, 1); - let writer = WalWriter::create(&vfs, &wal_path, header).unwrap(); + let mut writer = WalWriter::create(&vfs, &wal_path, header).unwrap(); writer.sync().unwrap(); let result = checkpoint_passive(&vfs, &db_path, 512).unwrap(); diff --git a/src/pager/wal.rs b/src/pager/wal.rs index b004c7e1..62604256 100644 --- a/src/pager/wal.rs +++ b/src/pager/wal.rs @@ -454,6 +454,18 @@ pub struct WalWriter { header: WalHeader, running: (u32, u32), offset: u64, + /// Byte offset `pending` should be written at — the value `offset` had + /// before the first frame accumulated into `pending` since the last + /// [`WalWriter::sync`]. `None` while `pending` is empty. + pending_offset: Option, + /// Frames appended since the last [`WalWriter::sync`], accumulated + /// here instead of written immediately (#635): a multi-page commit + /// then costs one `write_at` covering every frame instead of one + /// `write_at` per dirty page, which dominated commit latency for + /// updates scattered across many leaf pages. Flushed to `file` by + /// `sync`, which still fsyncs exactly once per commit (ADR-0026 + /// unchanged — this only batches the writes feeding that one fsync). + pending: Vec, /// Reusable frame buffer for [`WalWriter::append_frame`] (#588): one /// allocation per writer instead of one per frame, while keeping the /// frame header + page as a single `write_at` (ADR-0026 unchanged). @@ -470,6 +482,8 @@ impl WalWriter { running: header.header_checksum, offset: HEADER_LEN as u64, header, + pending_offset: None, + pending: Vec::new(), scratch: Vec::new(), }) } @@ -506,15 +520,26 @@ impl WalWriter { .reserve(FRAME_HEADER_LEN.saturating_add(page_data.len())); self.scratch.extend_from_slice(&frame_header); self.scratch.extend_from_slice(page_data); - self.file.write_at(&self.scratch, self.offset)?; + + if self.pending_offset.is_none() { + self.pending_offset = Some(self.offset); + } + self.pending.extend_from_slice(&self.scratch); self.running = after_page; self.offset = self.offset.saturating_add(self.scratch.len() as u64); Ok(()) } - /// Flushes every frame written so far to durable storage. - pub fn sync(&self) -> Result<(), WalError> { + /// Writes every frame accumulated since the last call, in one + /// `write_at` covering the whole run, then flushes to durable storage. + /// A no-op `write_at`-wise (fsync still runs) when nothing is pending, + /// e.g. a second `sync()` call or a writer that appended no frames. + pub fn sync(&mut self) -> Result<(), WalError> { + if let Some(pending_offset) = self.pending_offset.take() { + self.file.write_at(&self.pending, pending_offset)?; + self.pending.clear(); + } self.file.sync()?; Ok(()) } @@ -549,6 +574,8 @@ impl WalWriter { header, running, offset, + pending_offset: None, + pending: Vec::new(), scratch: Vec::new(), }) }