Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/runtime/webcore/FileReader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<usize>,
}

impl Default for OpenedFileBlob {
Expand All @@ -117,6 +121,7 @@ impl Default for OpenedFileBlob {
nonblocking: true,
#[cfg(not(windows))]
file_type: FileType::File,
known_size: None,
}
}
}
Expand Down Expand Up @@ -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)`
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -312,6 +328,7 @@ impl FileReader {
let mut pollable = false;
#[cfg(unix)]
let mut file_type = FileType::File;
let mut known_size: Option<usize> = 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<Store>`
// is owned locally and the cell borrow is released immediately.
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions src/runtime/webcore/blob/read_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
67 changes: 66 additions & 1 deletion test/js/bun/util/bun-file-read.test.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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);
});
});
Loading