file_sector_source: restore read-side DONTNEED + SEQUENTIAL (the actual fix)

Empirical: isolated NFS read 70 MB/s + write 93 MB/s on the rip1 setup
right now, but mux throughput pinned at 2.7 MB/s on 0.21.5. NOT
environmental — code regression.

Root cause: Phase 1 silently dropped the read-side
posix_fadvise(POSIX_FADV_DONTNEED) eviction that the pre-Phase-1 (0.20.7)
hot path had. Without it, an 85 GB streaming ISO read pins the entire
file in the kernel page cache, starving concurrent MKV writeback. 0.21.2
then also dropped the POSIX_FADV_SEQUENTIAL hint on the same theory,
compounding the regression.

Restored both, per-OS split:
- linux: posix_fadvise(SEQUENTIAL) at open + posix_fadvise(DONTNEED)
  on consumed 32 MiB windows
- macos: F_RDADVISE hint at open (kept); drop_window no-op (macOS unified
  buffer cache less prone to the pin pathology)
- windows / other: both no-op stubs

Target mux speed restored to 20+ MB/s (per concurrent-NFS math:
70/2 read × 0.73 MKV/ISO ratio ≈ 25 MB/s achievable).
This commit is contained in:
MattJackson
2026-05-14 09:16:23 -07:00
parent a383200ef1
commit e3b2c9d850
5 changed files with 91 additions and 13 deletions
+41 -13
View File
@@ -1,18 +1,46 @@
//! Linux: kernel readahead hint for the ISO file. //! Linux read-side platform hooks: sequential-access hint at open +
//! periodic page-cache eviction during streaming reads.
//! //!
//! Originally `POSIX_FADV_SEQUENTIAL` to widen the readahead window. //! ## Why both
//! On NFS that turned out to cause aggressive multi-MB readahead bursts //!
//! that saturated the TCP connection and starved concurrent writes //! `POSIX_FADV_SEQUENTIAL` at open widens the kernel's readahead window
//! during mux — observed empirically as a ~3× drop in mux throughput //! so each pread aggregates into fewer NFS round-trips. `DONTNEED` on
//! on the rip1/unraid-1 setup (0.21.0 vs 0.20.7 baseline). The kernel's //! the consumed window (called periodically by the caller) drops the
//! default readahead (~128 KiB on Linux) interleaves more naturally //! already-read pages from the page cache so an 85 GB streaming ISO
//! with the muxer's concurrent NFS writes, so we no longer issue any //! read doesn't fill memory and starve concurrent writes (the MKV
//! hint here. The per-OS file stays so the convention is honoured and //! output during mux). Together they mirror the write-side
//! we can re-enable a hint cleanly if a different storage path benefits. //! WritebackPipeline's policy.
//!
//! ## History
//!
//! Pre-Phase-1 (0.20.7 baseline) had both. Phase 1's introduction of
//! `FileSectorSource` silently dropped the read-side DONTNEED, and
//! 0.21.2's revert of `SEQUENTIAL` (mistakenly attributing a regression
//! to it) removed the hint. Net effect: 85 GB of ISO reads pinned in
//! the page cache + no readahead widening → mux throughput collapse
//! from 18 MB/s historical to 2.7-8 MB/s on 0.21.x. Restored in 0.21.6.
use std::fs::File; use std::fs::File;
use std::os::unix::io::AsRawFd;
pub(super) fn hint_sequential(_file: &File, _len_bytes: u64) { pub(super) fn hint_sequential(file: &File, _len_bytes: u64) {
// No-op: see module-level comment. Kernel default readahead is // Best-effort: return value ignored. A fadvise failure has no
// what we want on NFS-backed ISOs, which is the dominant case. // user-observable consequence.
unsafe {
libc::posix_fadvise(file.as_raw_fd(), 0, 0, libc::POSIX_FADV_SEQUENTIAL);
}
}
/// Drop pages in the half-open byte range `[start, start+len)` from
/// the page cache. Called periodically by `read_sectors` to bound the
/// read-side page cache pressure.
pub(super) fn drop_window(file: &File, start: u64, len: u64) {
unsafe {
libc::posix_fadvise(
file.as_raw_fd(),
start as i64,
len as i64,
libc::POSIX_FADV_DONTNEED,
);
}
} }
+8
View File
@@ -36,3 +36,11 @@ pub(super) fn hint_sequential(file: &File, len_bytes: u64) {
libc::fcntl(file.as_raw_fd(), F_RDADVISE, &mut ra); libc::fcntl(file.as_raw_fd(), F_RDADVISE, &mut ra);
} }
} }
/// macOS has no direct `POSIX_FADV_DONTNEED` equivalent for a byte
/// range. `fcntl(F_NOCACHE)` would disable caching globally on the fd
/// (too coarse — we want the unread region to still benefit). Best
/// approximation: no-op. macOS's unified buffer cache is generally
/// less prone to the pin-everything pathology that triggers the
/// regression on Linux NFS clients.
pub(super) fn drop_window(_file: &File, _start: u64, _len: u64) {}
+35
View File
@@ -87,6 +87,17 @@ const BUF_SECTORS: u32 = (READAHEAD_BUF_BYTES / SECTOR_SIZE) as u32;
/// `read_sectors` is satisfied from the buffer when possible; otherwise /// `read_sectors` is satisfied from the buffer when possible; otherwise
/// a full-buffer refill is issued at the requested LBA's position and /// a full-buffer refill is issued at the requested LBA's position and
/// the call is re-tried against the freshly populated window. /// the call is re-tried against the freshly populated window.
/// Bytes-read threshold per `posix_fadvise(DONTNEED)` drop on the
/// read side. Mirrors `WRITEBACK_CHUNK_BYTES` so the read-side page
/// cache stays bounded the same way the write side does.
///
/// 0.21.6: re-added after empirical discovery that Phase 1 had silently
/// dropped this from the pre-Phase-1 (0.20.7) hot path. Without it,
/// 85 GB of streaming ISO reads pin the entire file in the kernel page
/// cache, starving the MKV writeback and collapsing mux throughput
/// (observed: 2.7 MB/s mux on 0.21.5 vs. 70 MB/s isolated NFS reads).
const READ_DROP_CHUNK_BYTES: u64 = 32 * 1024 * 1024;
pub struct FileSectorSource { pub struct FileSectorSource {
file: File, file: File,
/// Total file size in sectors. Constant after construction; /// Total file size in sectors. Constant after construction;
@@ -102,6 +113,13 @@ pub struct FileSectorSource {
#[allow(dead_code)] #[allow(dead_code)]
buf_start_lba: u32, buf_start_lba: u32,
buf_len_sectors: u32, buf_len_sectors: u32,
/// 0.21.6: bytes read since the last DONTNEED drop. Drives the
/// per-`READ_DROP_CHUNK_BYTES` page-cache eviction in read_sectors.
bytes_read_since_drop: u64,
/// 0.21.6: file offset at which the current drop window starts.
/// The next DONTNEED drops from `drop_window_start` for
/// `bytes_read_since_drop` bytes.
drop_window_start: u64,
} }
impl FileSectorSource { impl FileSectorSource {
@@ -142,6 +160,8 @@ impl FileSectorSource {
buf, buf,
buf_start_lba: 0, buf_start_lba: 0,
buf_len_sectors: 0, buf_len_sectors: 0,
bytes_read_since_drop: 0,
drop_window_start: 0,
}) })
} }
@@ -228,6 +248,21 @@ impl SectorSource for FileSectorSource {
.read_exact(&mut out[..bytes]) .read_exact(&mut out[..bytes])
.map_err(|e| Error::IoError { source: e })?; .map_err(|e| Error::IoError { source: e })?;
self.buf_len_sectors = 0; self.buf_len_sectors = 0;
// 0.21.6: periodic page-cache eviction on the read side. Without
// this, an 85 GB streaming ISO read pins the entire file in
// kernel page cache, which starves concurrent NFS writes (the
// MKV output) and collapses mux throughput. Mirrors the
// write-side WritebackPipeline's DONTNEED policy.
self.bytes_read_since_drop += bytes as u64;
if self.bytes_read_since_drop >= READ_DROP_CHUNK_BYTES {
let drop_start = self.drop_window_start;
let drop_len = self.bytes_read_since_drop;
platform::drop_window(&self.file, drop_start, drop_len);
self.drop_window_start = drop_start + drop_len;
self.bytes_read_since_drop = 0;
}
Ok(bytes) Ok(bytes)
} }
} }
+2
View File
@@ -5,3 +5,5 @@
use std::fs::File; use std::fs::File;
pub(super) fn hint_sequential(_file: &File, _len_bytes: u64) {} pub(super) fn hint_sequential(_file: &File, _len_bytes: u64) {}
pub(super) fn drop_window(_file: &File, _start: u64, _len: u64) {}
+5
View File
@@ -17,3 +17,8 @@ pub(super) fn hint_sequential(_file: &File, _len_bytes: u64) {
"FileSectorSource hint_sequential: windows stub (TODO: FILE_FLAG_SEQUENTIAL_SCAN at open)" "FileSectorSource hint_sequential: windows stub (TODO: FILE_FLAG_SEQUENTIAL_SCAN at open)"
); );
} }
/// Windows page-cache eviction is not exposed via a posix_fadvise
/// equivalent. The kernel does its own working-set management. No-op
/// for now.
pub(super) fn drop_window(_file: &File, _start: u64, _len: u64) {}