Skip to main content

max / audiofiles

Hoist the import's directory fsync to the end of the batch Measured 2026-07-29 (wiki af-benchmarks): the parent-directory sync_all ran once per imported file, on top of the temp-file sync_all in copy_hashing, so the blob path cost about two fsyncs per file. A probe with that one directory fsync disabled ran 40,000 files in 1336s against 1829s for shipped code on the identical corpus, batch size and drive -- roughly 27% of import wall time. Not the main bottleneck; the cheap follow-on to the fanout, which is already in. What it was buying is the question, and the answer is: less than it looked. Atomicity comes from the temp write plus the rename plus copy_hashing's content-address verification, not from this. All it buys is crash durability of the dirent, and a lost dirent self-heals -- needs_write treats a missing or size-mismatched blob as re-writable. The DB row it pairs with is not durable per-file either, under WAL and synchronous=NORMAL. So the per-file fsync was making a guarantee no other part of the path makes. The store now records the shard directories it has renamed into and fsyncs each once, at the end of the run, beside the WAL checkpoint the import already does. layout::migrate_to_sharded has always worked this way; this is the import path catching up to it. The two entry points make opposite promises, and the test pins them: import_hashed is the batch path and leaves a flush owed, import is one-shot with no batch end to defer to and flushes itself, so forge, export and the direct backend keep exactly today's durability. A caller that loops import_hashed and forgets the flush degrades to OS-timed dirent durability, which is what the sync download path has always had. While in here: the streaming buffer was 8 KiB in all four hash/copy passes, about 130 read syscalls per megabyte against a sequential read the kernel is already reading ahead for. Now 256 KiB, on the heap since a rayon worker's stack is not the place for it. Ratios not re-measured -- that wants an idle fw13, which is the benchmark task's last open item.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-16 17:23 UTC
Signed with PGP, not checked
Commit: 1a0edd03ac43821dcf4ec363ebdb099eac401d68
Parent: 4481d97
2 files changed, +138 insertions, -16 deletions
M crates/audiofiles-browser/src/import.rs +6
@@ -707,6 +707,12 @@
707 707
708 708 let total_files = if cancelled { completed } else { total };
709 709
710 + // The batch end the per-file directory fsync was hoisted out of: every
711 + // shard directory this run wrote a blob into gets its one fsync here.
712 + // Reached on the cancelled path too, since a cancelled import has still
713 + // landed every blob it got through.
714 + store.flush_dirs();
715 +
710 716 // Checkpoint WAL after large import to keep -shm file fresh
711 717 // and avoid stale memory-mapped state on macOS.
712 718 if let Err(e) = db.wal_checkpoint() {
M crates/audiofiles-core/src/store/mod.rs +132 -16
@@ -9,9 +9,11 @@
9 9 //! - **Cloud eviction:** Setting `cloud_only=true` deletes the local blob while keeping the
10 10 //! metadata row. The hash lets SyncKit re-download the exact file from blob storage later.
11 11
12 + use std::collections::BTreeSet;
12 13 use std::fs;
13 14 use std::io::{Read, Write};
14 15 use std::path::{Path, PathBuf};
16 + use std::sync::Mutex;
15 17
16 18 use sha2::{Digest, Sha256};
17 19 use symphonia::core::formats::probe::Hint;
@@ -103,6 +105,15 @@
103 105 })
104 106 }
105 107
108 + /// Buffer size for the streaming hash and copy passes.
109 + ///
110 + /// 8 KiB was the default and meant roughly 130 read syscalls per megabyte, all
111 + /// of them against a sequential read the kernel is already reading ahead for.
112 + /// At import scale that is a syscall count and nothing else. Heap-allocated
113 + /// rather than an array: 256 KiB is more than belongs on a rayon worker's
114 + /// stack, and the allocation is once per file against reading the whole file.
115 + const STREAM_BUF: usize = 256 * 1024;
116 +
106 117 /// Mark a stored blob read-only (best-effort, cross-platform). A failed chmod
107 118 /// must never fail an import, it only weakens the write-through guard.
108 119 fn set_blob_readonly(path: &Path) {
@@ -124,10 +135,13 @@
124 135 /// Deduplication via SHA-256: import streams the file through a hasher and skips
125 136 /// the copy if a blob with the same hash already exists.
126 137 ///
127 - /// Thread-safe: all operations are stateless reads/writes against the filesystem
128 - /// (no interior mutability or locks).
138 + /// Thread-safe. Every operation is a read/write against the filesystem; the one
139 + /// piece of state is [`pending_dirs`](Self::flush_dirs), behind a mutex.
129 140 pub struct SampleStore {
130 141 root: PathBuf,
142 + /// Shard directories written into whose fsync has been deferred to the end
143 + /// of the batch. See [`flush_dirs`](Self::flush_dirs) for why.
144 + pending_dirs: Mutex<BTreeSet<PathBuf>>,
131 145 }
132 146
133 147 impl SampleStore {
@@ -136,24 +150,86 @@
136 150 pub fn new(root: impl Into<PathBuf>) -> Result<Self> {
137 151 let root = root.into();
138 152 fs::create_dir_all(&root).map_err(|e| io_err(&root, e))?;
139 - Ok(Self { root })
153 + Ok(Self {
154 + root,
155 + pending_dirs: Mutex::new(BTreeSet::new()),
156 + })
140 157 }
141 158
142 159 /// Import a file into the store: hash it, copy to content-addressed path,
143 160 /// insert into DB. Returns the hex SHA-256 hash.
161 + ///
162 + /// The one-shot entry point, so it flushes its own directory fsync before
163 + /// returning: there is no batch end to defer to. [`import_hashed`] is the
164 + /// batch path and defers; see [`flush_dirs`](Self::flush_dirs).
165 + ///
166 + /// [`import_hashed`]: SampleStore::import_hashed
144 167 #[instrument(skip_all)]
145 168 pub fn import(&self, path: &Path, db: &Database) -> Result<String> {
146 169 let (hash, file_size) = hash_file(path)?;
147 170 let hash = SampleHash::from_trusted(hash);
148 - self.import_hashed(path, &hash, file_size, db)?;
171 + let result = self.import_hashed(path, &hash, file_size, db);
172 + self.flush_dirs();
173 + result?;
149 174 Ok(hash.into_inner())
150 175 }
151 176
177 + /// Fsync every shard directory written into since the last call, making the
178 + /// renames that landed blobs in them durable. Best-effort throughout: not
179 + /// every filesystem supports a directory fsync, and a failure here weakens
180 + /// durability without touching correctness.
181 + ///
182 + /// Called at the end of an import run rather than per file. Measured
183 + /// 2026-07-29 (wiki `af-benchmarks`), the per-file version was about 27% of
184 + /// import wall time: 40,000 files ran in 1336s with it disabled against
185 + /// 1829s shipped, on the identical corpus, batch size and drive.
186 + ///
187 + /// What the fsync was NOT buying, which is why the batch end is soon enough:
188 + /// atomicity here comes from the temp-file write plus the rename plus the
189 + /// content-address verification in [`copy_hashing`], not from this. All this
190 + /// buys is crash durability of the dirent, and a blob whose dirent is lost
191 + /// self-heals, `needs_write` treats a missing or size-mismatched blob as
192 + /// re-writable on the next import. The DB row it pairs with is not durable
193 + /// per-file either, the connection runs WAL + `synchronous=NORMAL`.
194 + ///
195 + /// [`layout::migrate_to_sharded`] already worked this way, one fsync for a
196 + /// sweep that renames every blob in the vault; this is the import path
197 + /// catching up to it.
198 + pub fn flush_dirs(&self) {
199 + let pending = std::mem::take(&mut *self.pending_dirs());
200 + for dir in pending {
201 + if let Ok(d) = fs::File::open(&dir) {
202 + let _ = d.sync_all();
203 + }
204 + }
205 + }
206 +
207 + /// Record that `dir` has had a blob renamed into it and wants an fsync at
208 + /// the next [`flush_dirs`](Self::flush_dirs).
209 + fn mark_dir_pending(&self, dir: &Path) {
210 + self.pending_dirs().insert(dir.to_path_buf());
211 + }
212 +
213 + /// The pending-directory set, recovering from a poisoned lock rather than
214 + /// panicking: the set is a to-do list of fsyncs, so a thread that died
215 + /// holding it leaves nothing inconsistent behind, and refusing to import
216 + /// afterwards would be the larger failure.
217 + fn pending_dirs(&self) -> std::sync::MutexGuard<'_, BTreeSet<PathBuf>> {
218 + self.pending_dirs
219 + .lock()
220 + .unwrap_or_else(std::sync::PoisonError::into_inner)
221 + }
222 +
152 223 /// Import a file whose SHA-256 hash and size were already computed (e.g. by a
153 224 /// parallel pre-hash pass). Does the serial side-effecting work, content-
154 225 /// addressed blob copy + DB insert + orphan cleanup, exactly as [`import`].
155 226 /// Splitting the pure hash out lets the import pipeline hash a batch in
156 227 /// parallel while keeping every store/DB mutation serial.
228 + ///
229 + /// The batch path, so it defers its directory fsync: a caller running this
230 + /// in a loop owes a [`flush_dirs`](Self::flush_dirs) at the end of the run.
231 + /// Skipping it is not a correctness failure, only a dirent left as durable
232 + /// as the OS makes it.
157 233 #[instrument(skip_all)]
158 234 pub fn import_hashed(
159 235 &self,
@@ -241,14 +317,13 @@
241 317 let _ = fs::remove_file(&tmp);
242 318 return Err(io_err(&dest, e));
243 319 }
244 - // Best-effort fsync of the directory so the rename itself is durable
245 - // (the new dirent survives a crash). Not all filesystems support
246 - // directory fsync; failure here only weakens durability, never the
247 - // import.
248 - if let Some(parent) = dest.parent()
249 - && let Ok(d) = fs::File::open(parent)
250 - {
251 - let _ = d.sync_all();
320 + // The rename wants a directory fsync to be durable (the new dirent
321 + // has to survive a crash), but not one per file: that cost about
322 + // 27% of import wall time and bought a guarantee no other part of
323 + // this path makes. The directory is recorded and fsynced once at
324 + // the end of the batch instead. See `flush_dirs`.
325 + if let Some(parent) = dest.parent() {
326 + self.mark_dir_pending(parent);
252 327 }
253 328 // Mark the canonical blob read-only. The VFS mirror exposes store
254 329 // blobs to DAWs/file managers via symlinks; a "save in place" would
@@ -515,7 +590,7 @@
515 590 let mut file = fs::File::open(&path).map_err(|e| io_err(&path, e))?;
516 591
517 592 let mut hasher = Sha256::new();
518 - let mut buf = [0u8; 8192];
593 + let mut buf = vec![0u8; STREAM_BUF];
519 594 loop {
520 595 let n = file.read(&mut buf).map_err(|e| io_err(&path, e))?;
521 596 if n == 0 {
@@ -922,7 +997,7 @@
922 997 // Verify hash matches
923 998 let mut file = fs::File::open(new_path).map_err(|e| io_err(new_path, e))?;
924 999 let mut hasher = Sha256::new();
925 - let mut buf = [0u8; 8192];
1000 + let mut buf = vec![0u8; STREAM_BUF];
926 1001 loop {
927 1002 let n = file.read(&mut buf).map_err(|e| io_err(new_path, e))?;
928 1003 if n == 0 {
@@ -980,7 +1055,7 @@
980 1055 }
981 1056
982 1057 let mut hasher = Sha256::new();
983 - let mut buf = [0u8; 8192];
1058 + let mut buf = vec![0u8; STREAM_BUF];
984 1059 loop {
985 1060 let n = file.read(&mut buf).map_err(|e| io_err(path, e))?;
986 1061 if n == 0 {
@@ -1001,7 +1076,7 @@
1001 1076 let mut input = fs::File::open(src).map_err(|e| io_err(src, e))?;
1002 1077 let mut output = fs::File::create(dst).map_err(|e| io_err(dst, e))?;
1003 1078 let mut hasher = Sha256::new();
1004 - let mut buf = [0u8; 8192];
1079 + let mut buf = vec![0u8; STREAM_BUF];
1005 1080 loop {
1006 1081 let n = input.read(&mut buf).map_err(|e| io_err(src, e))?;
1007 1082 if n == 0 {
@@ -1392,6 +1467,47 @@
1392 1467 assert_eq!(count, 1);
1393 1468 }
1394 1469
1470 + /// The two entry points make opposite promises about the directory fsync,
1471 + /// and the split is the whole of the 27% the hoist bought: `import_hashed`
1472 + /// is the batch path and leaves its shard directory owing a flush, while
1473 + /// `import` is one-shot and owes nothing on return.
1474 + ///
1475 + /// Pins the bookkeeping rather than the fsync, which is not observable. The
1476 + /// regression it guards is a caller (or a future one) that loops
1477 + /// `import_hashed` and never flushes, or an `import` that quietly starts
1478 + /// deferring on behalf of callers that have no batch end.
1479 + #[test]
1480 + fn the_batch_path_defers_its_directory_fsync_and_the_one_shot_path_does_not() {
1481 + let (dir, db, store) = setup();
1482 +
1483 + let batched = create_test_file(&dir, "batched.wav", b"batched bytes");
1484 + let (hash, size) = hash_file(&batched).unwrap();
1485 + let hash = crate::SampleHash::from_trusted(hash);
1486 + store.import_hashed(&batched, &hash, size, &db).unwrap();
1487 +
1488 + assert_eq!(
1489 + store.pending_dirs().len(),
1490 + 1,
1491 + "the batch path leaves its shard directory owing an fsync",
1492 + );
1493 + // Deferring the fsync must not defer the blob: it is renamed into place
1494 + // and readable now, which is why the batch end is soon enough.
1495 + assert!(store.exists(&hash, "wav").unwrap());
1496 +
1497 + store.flush_dirs();
1498 + assert!(
1499 + store.pending_dirs().is_empty(),
1500 + "flush_dirs drains the set, so a second run does not re-sync the world",
1501 + );
1502 +
1503 + let one_shot = create_test_file(&dir, "one_shot.wav", b"one-shot bytes");
1504 + store.import(&one_shot, &db).unwrap();
1505 + assert!(
1506 + store.pending_dirs().is_empty(),
1507 + "the one-shot path has no batch end to defer to, so it flushes itself",
1508 + );
1509 + }
1510 +
1395 1511 #[test]
1396 1512 fn import_deduplicates() {
1397 1513 let (dir, db, store) = setup();