[libc++] Revert fstream::read optimizations

[rupprecht]

This causes various runtime failures, as reported in #168628.

This reverts both #165223 and #167779

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Jordan Rupprecht (rupprecht)

Changes

This causes various runtime failures, as reported in #168628.

This reverts both #165223 and #167779

---

Full diff: https://github.com/llvm/llvm-project/pull/168894.diff

5 Files Affected:

• (modified) libcxx/docs/ReleaseNotes/22.rst (+2-2)
• (modified) libcxx/include/fstream (-19)
• (renamed) libcxx/test/benchmarks/streams/ofstream.bench.cpp (+2-20)
• (modified) libcxx/test/libcxx/input.output/file.streams/fstreams/filebuf/traits_mismatch.verify.cpp (+1-1)
• (modified) libcxx/test/libcxx/input.output/file.streams/fstreams/traits_mismatch.verify.cpp (+1-1)

diff --git a/libcxx/docs/ReleaseNotes/22.rst b/libcxx/docs/ReleaseNotes/22.rst
index 7af1351e8cee6..b8e0e9b5a1814 100644
--- a/libcxx/docs/ReleaseNotes/22.rst
+++ b/libcxx/docs/ReleaseNotes/22.rst
@@ -67,8 +67,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`` and ``ifstream::read`` have been optimized to pass through large reads and writes to system calls
-  directly instead of copying them in chunks.
+- ``ofstream::write`` has been optimized to pass through large strings to system calls directly instead of copying them
+  in chunks into a buffer.
 - 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 90e35740c17cf..1f88d134fe061 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -308,25 +308,6 @@ 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);
-      }
-      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/fstream.bench.cpp b/libcxx/test/benchmarks/streams/ofstream.bench.cpp
similarity index 54%
rename from libcxx/test/benchmarks/streams/fstream.bench.cpp
rename to libcxx/test/benchmarks/streams/ofstream.bench.cpp
index 3ca1801ca8d03..60606a9d67e2f 100644
--- a/libcxx/test/benchmarks/streams/fstream.bench.cpp
+++ b/libcxx/test/benchmarks/streams/ofstream.bench.cpp
@@ -11,7 +11,7 @@
 
 #include <benchmark/benchmark.h>
 
-static void bm_ofstream_write(benchmark::State& state) {
+static void bm_write(benchmark::State& state) {
   std::vector<char> buffer;
   buffer.resize(16384);
 
@@ -20,24 +20,6 @@ static void bm_ofstream_write(benchmark::State& state) {
   for (auto _ : state)
     stream.write(buffer.data(), buffer.size());
 }
-BENCHMARK(bm_ofstream_write);
-
-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);
+BENCHMARK(bm_write);
 
 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 30e7b66d42325..283adbc057d1e 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@*:* 11 {{only virtual member functions can be marked 'override'}}
+// expected-error@*:* 10 {{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 daafb36f9151a..ba6f3c31d3e34 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@*:* 13 {{only virtual member functions can be marked 'override'}}
+// expected-error@*:* 12 {{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.

[rupprecht]

Proposing this revert to keep trunk in a more stable state. Assuming CI passes, of course.

@philnik777 I don't know if you've seen #168628 -- maybe you already have a fix in progress.

[rupprecht]

Proposing this revert to keep trunk in a more stable state. Assuming CI passes, of course.

ARM tests on buildkite seem to be stuck on lack of available test runners, but everything else on CI is green. This should be safe to land if someone on libc++ can confirm this is OK to revert.