-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[libc++] Optimize fstream::read #165223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
[libc++] Optimize fstream::read #165223
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1c20a74 to
b3e6abe
Compare
Member
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesFull diff: https://github.com/llvm/llvm-project/pull/165223.diff 5 Files Affected:
diff --git a/libcxx/docs/ReleaseNotes/22.rst b/libcxx/docs/ReleaseNotes/22.rst
index 25d33a9c2eb50..b5376c31ae31c 100644
--- a/libcxx/docs/ReleaseNotes/22.rst
+++ b/libcxx/docs/ReleaseNotes/22.rst
@@ -64,8 +64,8 @@ Improvements and New Features
by up to 2.5x
- The performance of ``erase(iterator, iterator)`` in the unordered containers has been improved by up to 1.9x
- The performance of ``map::insert_or_assign`` has been improved by up to 2x
-- ``ofstream::write`` has been optimized to pass through large strings to system calls directly instead of copying them
- in chunks into a buffer.
+- ``ofstream::write`` and ``ifstream::read`` have been optimized to pass through large reads and writes to system calls
+ directly instead of copying them in chunks.
- Multiple internal types have been refactored to use ``[[no_unique_address]]``, resulting in faster compile times and
reduced debug information.
diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index 1f88d134fe061..b07ca636094af 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -308,6 +308,19 @@ protected:
return basic_streambuf<_CharT, _Traits>::xsputn(__str, __len);
}
+ _LIBCPP_HIDE_FROM_ABI_VIRTUAL streamsize xsgetn(char_type* __str, streamsize __len) override {
+ if (__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);
+ }
+ if (__len - __n >= this->egptr() - this->eback())
+ return std::fread(__str + __n, sizeof(char_type), __len - __n, __file_);
+ }
+ 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 62%
rename from libcxx/test/benchmarks/streams/ofstream.bench.cpp
rename to libcxx/test/benchmarks/streams/fstream.bench.cpp
index 60606a9d67e2f..3fef23506e67e 100644
--- a/libcxx/test/benchmarks/streams/ofstream.bench.cpp
+++ b/libcxx/test/benchmarks/streams/fstream.bench.cpp
@@ -22,4 +22,22 @@ static void bm_write(benchmark::State& state) {
}
BENCHMARK(bm_write);
+static void bm_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_read);
+
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.
|
Member
|
/libcxx-bot benchmark libcxx/test/benchmarks/streams/fstream.bench.cpp Benchmark results: |
ldionne
approved these changes
Nov 7, 2025
b3e6abe to
fc3c07b
Compare
vinay-deshmukh
pushed a commit
to vinay-deshmukh/llvm-project
that referenced
this pull request
Nov 8, 2025
``` Benchmark old new Difference % Difference ----------- -------- -------- ------------ -------------- bm_read 2468.45 736.27 -1732.18 -70.17% ```
jsji
reviewed
Nov 9, 2025
michaelrj-google
added a commit
to michaelrj-google/llvm-project
that referenced
this pull request
Nov 12, 2025
The optimized version of xsgetn for basic_filebuf added in llvm#165223 has an issue where if the reads come from both the buffer and the filesystem it returns the wrong number of characters. This patch should address the issue.
Sterling-Augustine
pushed a commit
that referenced
this pull request
Nov 13, 2025
The optimized version of xsgetn for basic_filebuf added in #165223 has an issue where if the reads come from both the buffer and the filesystem it returns the wrong number of characters. This patch should address the issue.
Collaborator
|
This also causes failures when reading from a default constructed |
rupprecht
added a commit
that referenced
this pull request
Nov 21, 2025
aadeshps-mcw
pushed a commit
to aadeshps-mcw/llvm-project
that referenced
this pull request
Nov 26, 2025
This causes various runtime failures, as reported in llvm#168628. This reverts both llvm#165223 and llvm#167779
Priyanshu3820
pushed a commit
to Priyanshu3820/llvm-project
that referenced
this pull request
Nov 26, 2025
This causes various runtime failures, as reported in llvm#168628. This reverts both llvm#165223 and llvm#167779
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.