[libcxx-commits] [libcxx] Reapply "[libc++] Optimize fstream::read" (PR #206453)
Nikolas Klauser via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Jun 29 03:41:26 PDT 2026
https://github.com/philnik777 created https://github.com/llvm/llvm-project/pull/206453
This was reverted due to causing crashes if an ifstream wasn't opened.
This patch addresses this issue by simply checking whether the `__file_`
handle is null, and if it is simply fall back to the generic
implementation.
Fixes #168628
Fixes #205845
This reverts commit 347512ff38748ac6ebfacbfda172edb5cf1edbe2.
>From b2551f79f7a70373f08d3e1d72405ea79b48ef0e Mon Sep 17 00:00:00 2001
From: Nikolas Klauser <nikolasklauser at berlin.de>
Date: Mon, 29 Jun 2026 12:22:36 +0200
Subject: [PATCH] Reapply "[libc++] Optimize fstream::read"
This was reverted due to causing crashes if an ifstream wasn't opened.
This patch addresses this issue by simply checking whether the `__file_`
handle is null, and if it is simply fall back to the generic
implementation.
Fixes #168628
Fixes #205845
This reverts commit 347512ff38748ac6ebfacbfda172edb5cf1edbe2.
---
libcxx/docs/ReleaseNotes/23.rst | 2 ++
libcxx/include/fstream | 19 ++++++++++++++++
.../{ofstream.bench.cpp => fstream.bench.cpp} | 22 +++++++++++++++++--
.../filebuf/traits_mismatch.verify.cpp | 2 +-
.../fstreams/traits_mismatch.verify.cpp | 2 +-
.../fstreams/ifstream.members/xsgetn.pass.cpp | 19 ++++++++++++++++
6 files changed, 62 insertions(+), 4 deletions(-)
rename libcxx/test/benchmarks/streams/{ofstream.bench.cpp => fstream.bench.cpp} (52%)
diff --git a/libcxx/docs/ReleaseNotes/23.rst b/libcxx/docs/ReleaseNotes/23.rst
index 37aa972d22ea3..66990e0855b36 100644
--- a/libcxx/docs/ReleaseNotes/23.rst
+++ b/libcxx/docs/ReleaseNotes/23.rst
@@ -69,6 +69,8 @@ Improvements and New Features
``"std::visit: variant is valueless"``, ``"std::get: variant is valueless"``, or
``"std::get: wrong alternative for variant"``. The standard only requires ``what()`` to return an
unspecified non-null string, so user code that does not match on the exact message remains correct.
+``ifstream::read`` has been optimized to pass through large reads to system calls directly instead of copying them in
+ chunks.
Deprecations and Removals
-------------------------
diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index 0c34c9149de0d..7b84bf6609086 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -309,6 +309,25 @@ protected:
return basic_streambuf<_CharT, _Traits>::xsputn(__str, __len);
}
+ _LIBCPP_HIDE_FROM_ABI_VIRTUAL streamsize xsgetn(char_type* __str, streamsize __len) override {
+ if (__file_ && __always_noconv_) {
+ const streamsize __n = std::min(this->egptr() - this->gptr(), __len);
+ if (__n != 0) {
+ traits_type::copy(__str, this->gptr(), __n);
+ this->__gbump_ptrdiff(__n);
+ }
+ const streamsize __remainder = __len - __n;
+ const streamsize __buffer_space = this->egptr() - this->eback();
+
+ if (__remainder >= __buffer_space)
+ return std::fread(__str + __n, sizeof(char_type), __remainder, __file_) + __n;
+ else if (__remainder > 0)
+ return basic_streambuf<_CharT, _Traits>::xsgetn(__str + __n, __remainder) + __n;
+ return __n;
+ }
+ return basic_streambuf<_CharT, _Traits>::xsgetn(__str, __len);
+ }
+
private:
char* __extbuf_;
const char* __extbufnext_;
diff --git a/libcxx/test/benchmarks/streams/ofstream.bench.cpp b/libcxx/test/benchmarks/streams/fstream.bench.cpp
similarity index 52%
rename from libcxx/test/benchmarks/streams/ofstream.bench.cpp
rename to libcxx/test/benchmarks/streams/fstream.bench.cpp
index 80827d59cdf01..2e16ae58749e6 100644
--- a/libcxx/test/benchmarks/streams/ofstream.bench.cpp
+++ b/libcxx/test/benchmarks/streams/fstream.bench.cpp
@@ -13,7 +13,7 @@
#include <benchmark/benchmark.h>
-static void bm_write(benchmark::State& state) {
+static void bm_ofstream_write(benchmark::State& state) {
std::vector<char> buffer;
buffer.resize(16384);
@@ -22,6 +22,24 @@ static void bm_write(benchmark::State& state) {
for (auto _ : state)
stream.write(buffer.data(), buffer.size());
}
-BENCHMARK(bm_write)->Name("std::ofstream::write(char*, size)");
+BENCHMARK(bm_ofstream_write)->Name("std::ofstream::write(char*, size)");
+
+static void bm_ifstream_read(benchmark::State& state) {
+ std::vector<char> buffer;
+ buffer.resize(16384);
+
+ std::ofstream gen_testfile("testfile");
+ gen_testfile.write(buffer.data(), buffer.size());
+
+ std::ifstream stream("testfile");
+ assert(stream);
+
+ for (auto _ : state) {
+ stream.read(buffer.data(), buffer.size());
+ benchmark::DoNotOptimize(buffer);
+ stream.seekg(0);
+ }
+}
+BENCHMARK(bm_ifstream_read)->Name("std::ifstream::read(char*, size)");
BENCHMARK_MAIN();
diff --git a/libcxx/test/libcxx/input.output/file.streams/fstreams/filebuf/traits_mismatch.verify.cpp b/libcxx/test/libcxx/input.output/file.streams/fstreams/filebuf/traits_mismatch.verify.cpp
index 283adbc057d1e..30e7b66d42325 100644
--- a/libcxx/test/libcxx/input.output/file.streams/fstreams/filebuf/traits_mismatch.verify.cpp
+++ b/libcxx/test/libcxx/input.output/file.streams/fstreams/filebuf/traits_mismatch.verify.cpp
@@ -19,4 +19,4 @@
std::basic_filebuf<char, std::char_traits<wchar_t> > f;
// expected-error-re@*:* {{static assertion failed{{.*}}traits_type::char_type must be the same type as CharT}}
-// expected-error@*:* 10 {{only virtual member functions can be marked 'override'}}
+// expected-error@*:* 11 {{only virtual member functions can be marked 'override'}}
diff --git a/libcxx/test/libcxx/input.output/file.streams/fstreams/traits_mismatch.verify.cpp b/libcxx/test/libcxx/input.output/file.streams/fstreams/traits_mismatch.verify.cpp
index ba6f3c31d3e34..daafb36f9151a 100644
--- a/libcxx/test/libcxx/input.output/file.streams/fstreams/traits_mismatch.verify.cpp
+++ b/libcxx/test/libcxx/input.output/file.streams/fstreams/traits_mismatch.verify.cpp
@@ -21,7 +21,7 @@ std::basic_fstream<char, std::char_traits<wchar_t> > f;
// expected-error-re@*:* {{static assertion failed{{.*}}traits_type::char_type must be the same type as CharT}}
// expected-error-re@*:* {{static assertion failed{{.*}}traits_type::char_type must be the same type as CharT}}
-// expected-error@*:* 12 {{only virtual member functions can be marked 'override'}}
+// expected-error@*:* 13 {{only virtual member functions can be marked 'override'}}
// FIXME: As of commit r324062 Clang incorrectly generates a diagnostic about mismatching
// exception specifications for types which are already invalid for one reason or another.
diff --git a/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/xsgetn.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/xsgetn.pass.cpp
index de17cd4d8e9d3..17615a4812d7d 100644
--- a/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/xsgetn.pass.cpp
+++ b/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/xsgetn.pass.cpp
@@ -68,6 +68,25 @@ int main(int, char**) {
test_buffer.resize(24);
assert(std::string(test_buffer.data(), 24) == "to test buffer behaviour");
}
+ { // Ensure that read fails gracefully with an unopened ifstream
+ // See https://llvm.org/PR168628
+ char buf[10];
+ std::ifstream ifs;
+
+ assert(!ifs.is_open());
+ assert(!ifs.bad());
+ assert(!ifs.fail());
+ assert(!ifs.eof());
+ assert(ifs.good());
+
+ ifs.read(buf, sizeof(buf));
+
+ assert(!ifs.is_open());
+ assert(!ifs.bad());
+ assert(ifs.fail());
+ assert(ifs.eof());
+ assert(!ifs.good());
+ }
return 0;
}
More information about the libcxx-commits
mailing list