io/file_sector_source: bypass app-level buffer — direct pread per call
The 32 MiB readahead window (0.21.0–0.21.1) regressed mux throughput on NFS bidirectional workloads vs the pre-Phase-1 0.20.7 baseline (18 -> 7-8 MB/s). The 0.21.2 4 MiB shrink made it worse (5-6 MB/s). Both signs point at the application-level buffer itself, not the size. This commit bypasses the buffer entirely on the read path — every read_sectors call seeks and pread()s direct to the file. That matches 0.20.7's hot path. Kernel readahead handles the policy; on NFS that interleaves naturally with concurrent writes on the same TCP connection. Buffer state fields and refill/buffer_covers are kept so the structure is preserved for a future per-source-type policy (e.g. a local-disk source where batched reads ARE beneficial), and so the existing tests still exercise that machinery.
This commit is contained in:
@@ -207,14 +207,20 @@ impl SectorSource for FileSectorSource {
|
|||||||
if count == 0 {
|
if count == 0 {
|
||||||
return Ok(0);
|
return Ok(0);
|
||||||
}
|
}
|
||||||
// Refill if the requested range isn't entirely buffered.
|
// 0.21.3: bypass the application-level buffer entirely.
|
||||||
// `buffer_covers` also handles the empty-buffer case
|
//
|
||||||
// (buf_len_sectors == 0).
|
// Empirically the 32 MiB readahead window (0.21.0–0.21.1) and the
|
||||||
if !self.buffer_covers(lba, count) {
|
// 4 MiB shrink (0.21.2) both regressed mux throughput vs the
|
||||||
// A request larger than the buffer itself can never fit;
|
// pre-Phase-1 0.20.7 baseline on NFS bidirectional workloads
|
||||||
// fall back to a one-shot direct pread for that pathological
|
// (sweep ~25 MB/s OK; mux dropped from 18 → 7-8 → 5-6 MB/s).
|
||||||
// case so callers can't deadlock the source.
|
// Direct pread per call lets the kernel's own readahead policy
|
||||||
if count > BUF_SECTORS {
|
// run, which interleaves naturally with concurrent NFS writes on
|
||||||
|
// the same TCP connection.
|
||||||
|
//
|
||||||
|
// Buffer fields are retained (currently unused on this path) so
|
||||||
|
// any future per-source policy can be reintroduced without
|
||||||
|
// re-plumbing structure. `refill` / `buffer_covers` are kept too
|
||||||
|
// (still exercised by the tests so the API contract is locked).
|
||||||
let offset = lba as u64 * SECTOR_SIZE as u64;
|
let offset = lba as u64 * SECTOR_SIZE as u64;
|
||||||
self.file
|
self.file
|
||||||
.seek(SeekFrom::Start(offset))
|
.seek(SeekFrom::Start(offset))
|
||||||
@@ -222,18 +228,7 @@ impl SectorSource for FileSectorSource {
|
|||||||
self.file
|
self.file
|
||||||
.read_exact(&mut out[..bytes])
|
.read_exact(&mut out[..bytes])
|
||||||
.map_err(|e| Error::IoError { source: e })?;
|
.map_err(|e| Error::IoError { source: e })?;
|
||||||
// Invalidate buffer state — we bypassed it, the
|
|
||||||
// window is no longer authoritative for this LBA.
|
|
||||||
self.buf_len_sectors = 0;
|
self.buf_len_sectors = 0;
|
||||||
return Ok(bytes);
|
|
||||||
}
|
|
||||||
self.refill(lba)?;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Slice the buffer at the requested LBA's offset within it.
|
|
||||||
let off_sectors = (lba - self.buf_start_lba) as usize;
|
|
||||||
let off_bytes = off_sectors * SECTOR_SIZE;
|
|
||||||
out[..bytes].copy_from_slice(&self.buf[off_bytes..off_bytes + bytes]);
|
|
||||||
Ok(bytes)
|
Ok(bytes)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user