From 2beacea83bda3bc87d232f5b2a1c784db2df0eed Mon Sep 17 00:00:00 2001 From: Matthew Jackson Date: Sun, 17 May 2026 09:35:07 -0700 Subject: [PATCH] iter12: fallocate without KEEP_SIZE + truncate_at_sync; chunk back to 32 MiB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit iter11 (128 MiB) crashed to 16.6 mean — chunk-size sweet spot is firmly 32 MiB. Locking in. iter12 hypothesis: each NFS write past the file's EOF triggers a server-side SETATTR (metadata commit) to update the file length. With ~62 GiB of MKV output and writes at chunk boundaries, that's thousands of SETATTRs per rip. By using plain fallocate (mode 0) the file is pre-extended to size_bytes up front; subsequent writes overwrite the pre-extended region with no metadata ops. Adds high_water tracking + truncate_at_sync so we ftruncate down to actual content size if the size hint was an overestimate. --- src/io/writeback_file/linux.rs | 28 +++++++++++----------- src/io/writeback_file/mod.rs | 44 +++++++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/io/writeback_file/linux.rs b/src/io/writeback_file/linux.rs index f7204a1..34daa47 100644 --- a/src/io/writeback_file/linux.rs +++ b/src/io/writeback_file/linux.rs @@ -1,9 +1,14 @@ //! Linux platform impl for [`super::WritebackFile`]. //! -//! - `preallocate`: `fallocate(FALLOC_FL_KEEP_SIZE)` — reserve extents -//! without growing the reported file size. Reduces extent -//! fragmentation on large sequential writes (mux output on NFS in -//! particular). +//! - `preallocate`: `fallocate(0)` — reserve extents AND extend the +//! reported file size up-front. iter12 (2026-05-17): switched from +//! `FALLOC_FL_KEEP_SIZE` to plain mode 0. With KEEP_SIZE the file's +//! reported length stayed at 0 and every write past the previous +//! EOF triggered an NFS SETATTR (server-side metadata commit) to +//! grow the file. With mode 0, the file is full-size from the +//! start; subsequent writes overwrite pre-extended region in place +//! with zero metadata ops. At end of mux, caller `ftruncate`s down +//! to actual content size if hint was an overestimate. //! - `durable_sync`: `fsync` wrapped in //! [`crate::io::bounded::bounded_syscall`] with a 60 s deadline so a //! wedged NFS server can't trap the calling thread indefinitely. @@ -17,16 +22,11 @@ use std::time::Duration; /// Best-effort: a non-zero rc is logged but not propagated, since the /// caller would just continue with the unreserved file anyway. pub(super) fn preallocate(file: &File, size_bytes: u64) { - // FALLOC_FL_KEEP_SIZE = 0x01 — keep the reported file size at 0 - // (writes grow it normally) while still pre-reserving the extents. - let rc = unsafe { - libc::fallocate( - file.as_raw_fd(), - libc::FALLOC_FL_KEEP_SIZE, - 0, - size_bytes as i64, - ) - }; + // Mode 0 (no KEEP_SIZE) — reserve extents AND extend the + // reported file size to `size_bytes`. On NFS this eliminates the + // per-write SETATTR that would otherwise fire each time writes + // crossed the previous EOF. + let rc = unsafe { libc::fallocate(file.as_raw_fd(), 0, 0, size_bytes as i64) }; tracing::debug!( target: "mux", "WritebackFile fallocate size_hint={size_bytes} rc={rc} ok={}", diff --git a/src/io/writeback_file/mod.rs b/src/io/writeback_file/mod.rs index 0995c98..e26bac1 100644 --- a/src/io/writeback_file/mod.rs +++ b/src/io/writeback_file/mod.rs @@ -73,18 +73,23 @@ use std::path::Path; use super::writeback::WritebackPipeline; /// Granularity at which the Linux writeback pipeline issues -/// `sync_file_range` pairs. -/// -/// iter11 (2026-05-17): 32 → 128 MiB. 0.21.14 tried this under Phase -/// 2.5 and reverted; with Phase 2.5 disabled (iter8 baseline) the -/// tradeoff is different. iter8 (32 MiB) = 28.7, iter9 (64 MiB) = 27.5. -/// Trying 128 to see if the iter9 dip was noise or a real trend. -const WRITEBACK_CHUNK_BYTES: u64 = 128 * 1024 * 1024; +/// `sync_file_range` pairs. 32 MiB is the empirically best value +/// (iter8: 28.7; iter9 64 MiB: 27.5; iter11 128 MiB: 16.6; iter6 +/// 8 MiB: 15.8). Locking in. +const WRITEBACK_CHUNK_BYTES: u64 = 32 * 1024 * 1024; pub(crate) struct WritebackFile { file: File, pipeline: WritebackPipeline, pos: u64, + /// Highest position ever reached by `write`/`write_all`. Used by + /// `sync_all` to truncate the file down to the actual content + /// extent if `preallocate` over-reserved. + high_water: u64, + /// True if the file was preallocated AND extended to a hint size + /// at construction. `sync_all` will `ftruncate` to `high_water` + /// when this is set, to discard any over-reservation. + truncate_at_sync: bool, } impl WritebackFile { @@ -99,6 +104,8 @@ impl WritebackFile { file, pipeline, pos, + high_water: pos, + truncate_at_sync: false, }) } @@ -131,7 +138,14 @@ impl WritebackFile { pub(crate) fn create_with_size_hint(path: &Path, size_bytes: u64) -> io::Result { let file = File::create(path)?; platform::preallocate(&file, size_bytes); - Self::new(file) + let mut wbf = Self::new(file)?; + // `preallocate` (on Linux/macOS where it's implemented) extends + // the file's reported size to `size_bytes`. We mark this so + // `sync_all` will truncate down to actual content extent at + // mux end. If the hint was an underestimate, writes simply + // extend past it as normal. + wbf.truncate_at_sync = true; + Ok(wbf) } /// Open an existing file at `path` for writing (no truncation) and @@ -154,6 +168,14 @@ impl WritebackFile { /// effort, but bounded. pub(crate) fn sync_all(&mut self) -> io::Result<()> { self.pipeline.finalize(); + if self.truncate_at_sync { + // Truncate down to the actual content extent. If + // `preallocate` extended past the muxer's real output + // size, the tail is otherwise zero-filled garbage. + // `set_len` is ftruncate; safe to call even when + // high_water == current file size (no-op). + self.file.set_len(self.high_water)?; + } platform::durable_sync(&self.file) } } @@ -162,6 +184,9 @@ impl Write for WritebackFile { fn write(&mut self, buf: &[u8]) -> io::Result { let n = self.file.write(buf)?; self.pos += n as u64; + if self.pos > self.high_water { + self.high_water = self.pos; + } self.pipeline.note_progress(self.pos); Ok(n) } @@ -169,6 +194,9 @@ impl Write for WritebackFile { fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { self.file.write_all(buf)?; self.pos += buf.len() as u64; + if self.pos > self.high_water { + self.high_water = self.pos; + } self.pipeline.note_progress(self.pos); Ok(()) }