Optimize File::read_to_end and read_to_string

[jkugelman]

Reading a file into an empty vector or string buffer can incur unnecessary read syscalls and memory re-allocations as the buffer "warms up" and grows to its final size. This is perhaps a necessary evil with generic readers, but files can be read in more intelligently by checking the file size and reserving that much capacity.

std::fs::read and std::fs::read_to_string already perform this optimization: they open the file, reads its metadata, and call with_capacity with the file size. This ensures that the buffer does not need to be resized and avoids an initial string of small read syscalls.

However, if a user opens the File themselves and calls file.read_to_end or file.read_to_string they do not get this optimization.

letmut buf = Vec::new();
file.read_to_end(&mut buf)?;

I searched through this project's codebase and even here there are a lot of examples of this. They're found all over in unit tests, which isn't a big deal, but there are also several real instances in the compiler and in Cargo. I've documented the ones I found in a comment here:

#89516 (comment)

Most telling, the documentation for both the Read trait and the Read::read_to_end method both show this exact pattern as examples of how to use readers. What this says to me is that this shouldn't be solved by simply fixing the instances of it in this codebase. If it's here it's certain to be prevalent in the wider Rust ecosystem.

To that end, this commit adds specializations of read_to_end and read_to_string directly on File. This way it's no longer a minor footgun to start with an empty buffer when reading a file in.

A nice side effect of this change is that code that accesses a File as impl Read or dyn Read will benefit. For example, this code from compiler/rustc_serialize/src/json.rs:

pubfn from_reader(rdr:&mutdynRead) -> Result<Json,BuilderError>{letmut contents = Vec::new();match rdr.read_to_end(&mut contents){

Related changes:

• I also added specializations to BufReader to delegate to self.inner's methods. That way it can call File's optimized implementations if the inner reader is a file.

• The private std::io::append_to_string function is now marked unsafe.

• File::read_to_string being more efficient means that the performance note for io::read_to_string can be softened. I've added @camelid's suggested wording from Tracking Issue for std::io::read_to_string #80218 (comment).

r? @joshtriplett

[rust-timer]

Insufficient permissions to issue commands to rust-timer.

[bors]

@jkugelman: 🔑 Insufficient privileges: not in try users

[rust-timer]

Insufficient permissions to issue commands to rust-timer.

[jkugelman]

I tested both read_to_end and read_to_string on a 10MB input file. It took 21 read syscalls to read it before, and now it only takes 2.

Before and after straces of read_to_end

use std::env;use std::fs::File;use std::io::Read;pubfnmain(){letmut file = File::open(env::args().nth(1).expect("no file name")).expect("open failed");letmut buf = Vec::new();
file.read_to_end(&mut buf).expect("read_to_end failed");}

Before:

$ strace -s 0 ./read_to_end large.txt
...
openat(AT_FDCWD, "large.txt", O_RDONLY|O_CLOEXEC) = 3
read(3, ""..., 32) = 32
read(3, ""..., 32) = 32
read(3, ""..., 64) = 64
read(3, ""..., 128) = 128
read(3, ""..., 256) = 256
read(3, ""..., 512) = 512
read(3, ""..., 1024) = 1024
read(3, ""..., 2048) = 2048
read(3, ""..., 4096) = 4096
read(3, ""..., 8192) = 8192
read(3, ""..., 16384) = 16384
read(3, ""..., 32768) = 32768
read(3, ""..., 65536) = 65536
mmap(NULL, 266240, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fbd9ad0b000
read(3, ""..., 131072) = 131072
mremap(0x7fbd9ad0b000, 266240, 528384, MREMAP_MAYMOVE) = 0x7fbd9ac8a000
read(3, ""..., 262144) = 262144
mremap(0x7fbd9ac8a000, 528384, 1052672, MREMAP_MAYMOVE) = 0x7fbd9ab89000
read(3, ""..., 524288) = 524288
mremap(0x7fbd9ab89000, 1052672, 2101248, MREMAP_MAYMOVE) = 0x7fbd9a988000
read(3, ""..., 1048576) = 1048576
mremap(0x7fbd9a988000, 2101248, 4198400, MREMAP_MAYMOVE) = 0x7fbd9a587000
read(3, ""..., 2097152) = 2097152
mremap(0x7fbd9a587000, 4198400, 8392704, MREMAP_MAYMOVE) = 0x7fbd99d86000
read(3, ""..., 4194304) = 4194304
mremap(0x7fbd99d86000, 8392704, 16781312, MREMAP_MAYMOVE) = 0x7fbd98d85000
read(3, ""..., 8388608) = 1711392
read(3, "", 6677216) = 0
munmap(0x7fbd98d85000, 16781312) = 0
close(3) = 0
...

After:

$ strace -s 0 ./read_to_end large.txt
...
openat(AT_FDCWD, "large.txt", O_RDONLY|O_CLOEXEC) = 3
futex(0x7fb36787f0c8, FUTEX_WAKE_PRIVATE, 2147483647) = 0
statx(0, NULL, AT_STATX_SYNC_AS_STAT, STATX_ALL, NULL) = -1 EFAULT (Bad address)
statx(3, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|0x1000, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=10100000, ...}) = 0
mmap(NULL, 10100736, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fb366ce4000
read(3, ""..., 10100000) = 10100000
read(3, "", 32) = 0
munmap(0x7fb366ce4000, 10100736) = 0
close(3) = 0
...

Before and after straces of read_to_string

use std::env;use std::fs::File;use std::io::Read;pubfnmain(){letmut file = File::open(env::args().nth(1).expect("no file name")).expect("open failed");letmut buf = String::new();
file.read_to_string(&mut buf).expect("read_to_string failed");}

Before:

$ strace -s 0 ./read_to_string large.txt
...
openat(AT_FDCWD, "large.txt", O_RDONLY|O_CLOEXEC) = 3
read(3, ""..., 32) = 32
read(3, ""..., 32) = 32
read(3, ""..., 64) = 64
read(3, ""..., 128) = 128
read(3, ""..., 256) = 256
read(3, ""..., 512) = 512
read(3, ""..., 1024) = 1024
read(3, ""..., 2048) = 2048
read(3, ""..., 4096) = 4096
read(3, ""..., 8192) = 8192
read(3, ""..., 16384) = 16384
read(3, ""..., 32768) = 32768
read(3, ""..., 65536) = 65536
mmap(NULL, 266240, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f0d54602000
read(3, ""..., 131072) = 131072
mremap(0x7f0d54602000, 266240, 528384, MREMAP_MAYMOVE) = 0x7f0d54581000
read(3, ""..., 262144) = 262144
mremap(0x7f0d54581000, 528384, 1052672, MREMAP_MAYMOVE) = 0x7f0d54480000
read(3, ""..., 524288) = 524288
mremap(0x7f0d54480000, 1052672, 2101248, MREMAP_MAYMOVE) = 0x7f0d5427f000
read(3, ""..., 1048576) = 1048576
mremap(0x7f0d5427f000, 2101248, 4198400, MREMAP_MAYMOVE) = 0x7f0d53e7e000
read(3, ""..., 2097152) = 2097152
mremap(0x7f0d53e7e000, 4198400, 8392704, MREMAP_MAYMOVE) = 0x7f0d5367d000
read(3, ""..., 4194304) = 4194304
mremap(0x7f0d5367d000, 8392704, 16781312, MREMAP_MAYMOVE) = 0x7f0d5267c000
read(3, ""..., 8388608) = 1711392
read(3, "", 6677216) = 0
munmap(0x7f0d5267c000, 16781312) = 0
close(3) = 0
...

After:

$ strace -s 0 ./read_to_string large.txt
...
openat(AT_FDCWD, "large.txt", O_RDONLY|O_CLOEXEC) = 3
futex(0x7f65ecb160c8, FUTEX_WAKE_PRIVATE, 2147483647) = 0
statx(0, NULL, AT_STATX_SYNC_AS_STAT, STATX_ALL, NULL) = -1 EFAULT (Bad address)
statx(3, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|0x1000, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=10100000, ...}) = 0
mmap(NULL, 10100736, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f65ebf7b000
read(3, ""..., 10100000) = 10100000
read(3, "", 32) = 0
munmap(0x7f65ebf7b000, 10100736) = 0
close(3) = 0
...

[joshtriplett]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit e5a65adb22f1984fc191b41966c9be0fc8596105 with merge 5b3e6dca0007e54ee39f29f8cce557d735716218...

[bors]

☀️ Try build successful - checks-actions
Build commit: 5b3e6dca0007e54ee39f29f8cce557d735716218 (5b3e6dca0007e54ee39f29f8cce557d735716218)

[rust-timer]

Queued 5b3e6dca0007e54ee39f29f8cce557d735716218 with parent 98a5a98, future comparison URL.

[rust-timer]

Finished benchmarking commit (5b3e6dca0007e54ee39f29f8cce557d735716218): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

[camelid]

Summary: This benchmark run did not return any relevant changes.

Note that the benchmark suite consists of running rustc and rustdoc on various files. They are not very Read-heavy, so it's unlikely the results accurately reflect this PR's perf effect.

[jkugelman]

@joshtriplett I think this is ready to go.

[joshtriplett]

@bors r+

[bors]

📌 Commit a990c76 has been approved by joshtriplett

[bors]

⌛ Testing commit a990c76 with merge 910692d...

[bors]

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing 910692d to master...

[rust-timer]

Finished benchmarking commit (910692d): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

[noproto]

This change (added to Nightly 14 days ago) created a regression in Rust that was able to track by bisecting nightly releases. My previously working project now segfaults on Rust Nightly. I'm working on developing a test case, and I'll create a new issue for it.