diff --git a/src/runtime/webcore/FileReader.rs b/src/runtime/webcore/FileReader.rs index c22703418fec..d2ca98a17403 100644 --- a/src/runtime/webcore/FileReader.rs +++ b/src/runtime/webcore/FileReader.rs @@ -107,6 +107,10 @@ pub(crate) struct OpenedFileBlob { pub(crate) nonblocking: bool, #[cfg(not(windows))] pub(crate) file_type: FileType, + /// Size taken from the open fd, for a regular file whose store was statted + /// before the open. The reader stops there, so it needs no extra read to + /// find EOF. + pub(crate) known_size: Option, } impl Default for OpenedFileBlob { @@ -117,6 +121,7 @@ impl Default for OpenedFileBlob { nonblocking: true, #[cfg(not(windows))] file_type: FileType::File, + known_size: None, } } } @@ -218,6 +223,9 @@ impl Lazy { if sys::S::ISREG(mode) { is_nonblocking = false; + if stat.st_size > 0 && file.seekable.is_some() { + this.known_size = usize::try_from(stat.st_size).ok(); + } } // pollable: `S.ISFIFO(mode) or S.ISSOCK(mode)` @@ -247,6 +255,14 @@ impl Lazy { } } + // No `fstat` runs at open on Windows, so ask for the size alone. + #[cfg(windows)] + if file.seekable == Some(true) { + if let Ok(size @ 1..) = sys::get_file_size(fd) { + this.known_size = usize::try_from(size).ok(); + } + } + this.fd = fd; Ok(this) @@ -312,6 +328,7 @@ impl FileReader { let mut pollable = false; #[cfg(unix)] let mut file_type = FileType::File; + let mut known_size: Option = None; // R-2: move the `Lazy` out of the cell up-front (it's reset to `None` // on every path through the original `if let` body) so the `RefPtr` // is owned locally and the cell borrow is released immediately. @@ -334,6 +351,7 @@ impl FileReader { debug_assert!(opened.fd.is_valid()); self.fd.set(opened.fd); pollable = opened.pollable; + known_size = opened.known_size; #[cfg(unix)] { file_type = opened.file_type; @@ -396,7 +414,14 @@ impl FileReader { unsafe { (*self.parent()).increment_count() }; self.waiting_for_on_reader_done.set(true); } - self.reader().set_limit(self.max_size); + let limit = match known_size { + Some(size) => { + let available = size.saturating_sub(self.start_offset.unwrap_or(0)); + Some(self.max_size.map_or(available, |max| max.min(available))) + } + None => self.max_size, + }; + self.reader().set_limit(limit); let start_result = if let Some(offset) = self.start_offset { self.reader() .start_file_offset(self.fd.get(), pollable, offset) diff --git a/src/runtime/webcore/blob/read_file.rs b/src/runtime/webcore/blob/read_file.rs index 1209524a46d4..ffc36fefbe35 100644 --- a/src/runtime/webcore/blob/read_file.rs +++ b/src/runtime/webcore/blob/read_file.rs @@ -726,6 +726,14 @@ impl ReadFile { SizeType::try_from((stat.st_size as i64).max(0).min(MAX_SIZE as i64)).unwrap(); if stat.st_size > 0 && !self.could_block { + // A store that was statted before this read holds a file of known + // size. Stop at this `fstat`'s size, so the read needs no extra + // `read()` to find EOF. + if self.file_store.seekable.is_some() { + self.max_length = self + .max_length + .min(self.total_size.saturating_sub(self.offset)); + } self.size = self.total_size.min(self.max_length); // read up to 4k at a time if // they didn't explicitly set a size and we're reading from something that's not a regular file @@ -1252,6 +1260,13 @@ impl<'a> ReadFileUV<'a> { log!("is_regular_file: {}", this.is_regular_file); if stat.size() > 0 && this.is_regular_file { + // keep in sync with resolveSizeAndLastModified: a store that was + // statted before this read stops at this `fstat`'s size. + if this.file_store.seekable.is_some() { + this.max_length = this + .max_length + .min(this.total_size.saturating_sub(this.offset)); + } this.size = this.total_size.min(this.max_length); } else if stat.size() == 0 && !this.is_regular_file { // read up to 4k at a time if they didn't explicitly set a size and diff --git a/test/js/bun/util/bun-file-read.test.ts b/test/js/bun/util/bun-file-read.test.ts index 40c6309a0c13..7452a1a7b480 100644 --- a/test/js/bun/util/bun-file-read.test.ts +++ b/test/js/bun/util/bun-file-read.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { tempDir } from "harness"; +import { bunEnv, bunExe, isLinux, tempDir } from "harness"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -54,3 +54,68 @@ describe("Bun.file read-loop target selection", () => { expect(Bun.hash(buf)).toBe(Bun.hash(bytes.subarray(start, end))); }); }); + +// A Bun.file() whose store was already statted is a file of known size. Its +// readers stop at the size from their own fstat. They do not issue one more +// read() to find EOF. /proc/self/io counts the read syscalls of the subprocess. +describe.skipIf(!isLinux)("Bun.file read syscalls for a file of known size", () => { + const fixture = /* js */ ` + import { readFileSync } from "node:fs"; + const file = process.argv.at(-1); + const io = () => /^syscr: (\\d+)$/m.exec(readFileSync("/proc/self/io", "utf8")); + if (!io()) { + console.log(JSON.stringify({ supported: false })); + process.exit(0); + } + const syscr = () => Number(io()[1]); + const expected = readFileSync(file, "utf8"); + const drain = async stream => { + const decoder = new TextDecoder(); + let out = ""; + for await (const chunk of stream) out += decoder.decode(chunk, { stream: true }); + return out + decoder.decode(); + }; + const N = 100; + async function perOp(op) { + for (let i = 0; i < 5; i++) await op(); + const before = syscr(); + let ok = true; + for (let i = 0; i < N; i++) ok = (await op()) === expected && ok; + return { ok, readsPerOp: Math.round((syscr() - before) / N) }; + } + const open = touch => { const f = Bun.file(file); touch(f); return f; }; + console.log(JSON.stringify({ + supported: true, + lastModifiedThenText: await perOp(() => open(f => f.lastModified).text()), + lastModifiedThenStream: await perOp(() => drain(open(f => f.lastModified).stream())), + sizeThenText: await perOp(() => open(f => f.size).text()), + sizeThenStream: await perOp(() => drain(open(f => f.size).stream())), + })); + `; + + it("one read() per operation, no EOF probe", async () => { + using dir = tempDir("bun-file-read-count", { "data.txt": Buffer.alloc(64 * 1024, "a").toString() }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture, path.join(String(dir), "data.txt")], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + const { supported, ...counts } = JSON.parse(stdout); + if (!supported) { + console.warn("/proc/self/io has no syscr on this kernel, nothing to count"); + return; + } + // One read() returns the whole 64 KiB. A second one per operation is the EOF probe. + expect(counts).toEqual({ + lastModifiedThenText: { ok: true, readsPerOp: 1 }, + lastModifiedThenStream: { ok: true, readsPerOp: 1 }, + sizeThenText: { ok: true, readsPerOp: 1 }, + sizeThenStream: { ok: true, readsPerOp: 1 }, + }); + expect(exitCode).toBe(0); + }); +});