[libcxx-commits] [libcxx] [libc++] Optimize ofstream::write (PR #123337)
Nikolas Klauser via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Feb 19 23:52:46 PST 2025
https://github.com/philnik777 updated https://github.com/llvm/llvm-project/pull/123337
>From 11bc68bcfc4a530345ccc46660d7f5bc5c598d82 Mon Sep 17 00:00:00 2001
From: Nikolas Klauser <nikolasklauser at berlin.de>
Date: Fri, 17 Jan 2025 13:26:38 +0100
Subject: [PATCH] [libc++] Optimize ofstream::write
---
libcxx/docs/ReleaseNotes/21.rst | 3 +
libcxx/include/fstream | 12 ++
.../benchmarks/streams/ofstream.bench.cpp | 25 ++++
.../filebuf/traits_mismatch.verify.cpp | 10 +-
.../fstreams/traits_mismatch.verify.cpp | 2 +-
.../filebuf.virtuals/seekoff.pass.cpp | 1 -
.../filebuf.virtuals/sputn_not_open.pass.cpp | 41 ++++++
.../filebuf.virtuals/sputn_seekoff.pass.cpp | 138 ++++++++++++++++++
8 files changed, 221 insertions(+), 11 deletions(-)
create mode 100644 libcxx/test/benchmarks/streams/ofstream.bench.cpp
create mode 100644 libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_not_open.pass.cpp
create mode 100644 libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_seekoff.pass.cpp
diff --git a/libcxx/docs/ReleaseNotes/21.rst b/libcxx/docs/ReleaseNotes/21.rst
index 2439360797023..096fab031cd98 100644
--- a/libcxx/docs/ReleaseNotes/21.rst
+++ b/libcxx/docs/ReleaseNotes/21.rst
@@ -46,6 +46,9 @@ Improvements and New Features
- The ``std::ranges::{copy, copy_n, copy_backward}`` algorithms have been optimized for ``std::vector<bool>::iterator``\s,
resulting in a performance improvement of up to 2000x.
+- ``ofstream::write`` has been optimized to pass through large strings to system calls directly instead of copying them
+ in chunks into a buffer.
+
- Updated formatting library to Unicode 16.0.0.
Deprecations and Removals
diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index de5c07035dba9..7028613ca189a 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -226,6 +226,8 @@ _LIBCPP_EXPORTED_FROM_ABI void* __filebuf_windows_native_handle(FILE* __file) no
template <class _CharT, class _Traits>
class _LIBCPP_TEMPLATE_VIS basic_filebuf : public basic_streambuf<_CharT, _Traits> {
+ using __base _LIBCPP_NODEBUG = basic_streambuf<_CharT, _Traits>;
+
public:
typedef _CharT char_type;
typedef _Traits traits_type;
@@ -298,6 +300,16 @@ protected:
int sync() override;
void imbue(const locale& __loc) override;
+ _LIBCPP_HIDE_FROM_ABI_VIRTUAL streamsize xsputn(const char_type* __str, streamsize __len) override {
+ if (__always_noconv_ && __len >= (this->epptr() - this->pbase())) {
+ if (traits_type::eq_int_type(overflow(), traits_type::eof()))
+ return 0;
+
+ return std::fwrite(__str, sizeof(char_type), __len, __file_);
+ }
+ return __base::xsputn(__str, __len);
+ }
+
private:
char* __extbuf_;
const char* __extbufnext_;
diff --git a/libcxx/test/benchmarks/streams/ofstream.bench.cpp b/libcxx/test/benchmarks/streams/ofstream.bench.cpp
new file mode 100644
index 0000000000000..60606a9d67e2f
--- /dev/null
+++ b/libcxx/test/benchmarks/streams/ofstream.bench.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include <fstream>
+#include <vector>
+
+#include <benchmark/benchmark.h>
+
+static void bm_write(benchmark::State& state) {
+ std::vector<char> buffer;
+ buffer.resize(16384);
+
+ std::ofstream stream("/dev/null");
+
+ for (auto _ : state)
+ stream.write(buffer.data(), buffer.size());
+}
+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 37ab176ea26a0..9df0cffa1d441 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
@@ -21,12 +21,4 @@
std::basic_filebuf<char, std::char_traits<wchar_t> > f;
// expected-error-re at streambuf:* {{static assertion failed{{.*}}traits_type::char_type must be the same type as CharT}}
-// expected-error at fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error at fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error at fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error at fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error at fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error at fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error at fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error at fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error at fstream:* {{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 f936d8db47af5..007af7004dfc6 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
@@ -23,7 +23,7 @@ std::basic_fstream<char, std::char_traits<wchar_t> > f;
// expected-error-re at ios:* {{static assertion failed{{.*}}traits_type::char_type must be the same type as CharT}}
// expected-error-re at streambuf:* {{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@*:* 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.
diff --git a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
index 8008901802e91..f6378e7998ee9 100644
--- a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
+++ b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
@@ -29,7 +29,6 @@ int main(int, char**)
| std::ios_base::trunc) != 0);
assert(f.is_open());
f.sputn("abcdefghijklmnopqrstuvwxyz", 26);
- LIBCPP_ASSERT(buf[0] == 'v');
pos_type p = f.pubseekoff(-15, std::ios_base::cur);
assert(p == 11);
assert(f.sgetc() == 'l');
diff --git a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_not_open.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_not_open.pass.cpp
new file mode 100644
index 0000000000000..257f00d0cbff7
--- /dev/null
+++ b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_not_open.pass.cpp
@@ -0,0 +1,41 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
+
+// <fstream>
+
+#include <cassert>
+#include <fstream>
+#include <vector>
+
+void sputn_not_open() {
+ std::vector<char> data(10, 'a');
+ std::filebuf f;
+ std::streamsize len = f.sputn(data.data(), data.size());
+ assert(len == 0);
+ assert(std::strncmp(data.data(), "aaaaaaaaaa", 10) == 0);
+}
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+void sputn_not_open_wchar() {
+ std::vector<wchar_t> data(10, L'a');
+ std::wfilebuf f;
+ std::streamsize len = f.sputn(data.data(), data.size());
+ assert(len == 0);
+ assert(std::wcsncmp(data.data(), L"aaaaaaaaaa", 10) == 0);
+}
+#endif
+
+int main(int, char **) {
+ sputn_not_open();
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+ sputn_not_open_wchar();
+#endif
+ return 0;
+}
\ No newline at end of file
diff --git a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_seekoff.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_seekoff.pass.cpp
new file mode 100644
index 0000000000000..e8d289004105f
--- /dev/null
+++ b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_seekoff.pass.cpp
@@ -0,0 +1,138 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
+// UNSUPPORTED: c++03
+
+// <fstream>
+
+#define TEST_HAS_NO_WIDE_CHARACTERS 1
+
+#include <algorithm>
+#include <cassert>
+#include <codecvt>
+#include <fstream>
+#include <locale>
+#include <vector>
+
+#include "platform_support.h"
+#include "test_macros.h"
+
+typedef std::filebuf::pos_type pos_type;
+typedef std::filebuf::off_type off_type;
+
+void sputn_seekoff(char* buf,
+ const size_t buf_size,
+ const std::streamsize chunk_size1,
+ const off_type offset1,
+ const std::streamsize chunk_size2) {
+ std::string data{"abcdefghijklmnopqrstuvwxyz"};
+ const std::streamsize data_size = static_cast<std::streamsize>(data.size());
+ assert(chunk_size1 <= data_size);
+ assert(chunk_size2 <= data_size);
+ // vector with expected data in the file to be written
+ std::size_t result_size = 5 + chunk_size1 + chunk_size2 + 1;
+ if (offset1 > 0) {
+ result_size += offset1;
+ }
+ std::vector<char> result(result_size, 0);
+ {
+ std::filebuf f;
+ f.pubsetbuf(buf, buf_size);
+ assert(f.open("sputn_seekoff.dat", std::ios_base::out) != 0);
+ assert(f.is_open());
+
+ assert(f.pubseekoff(off_type(5), std::ios_base::beg) = off_type(5));
+
+ std::vector<char> chunk(data.begin() + 5, data.begin() + 5 + chunk_size1);
+ std::copy(chunk.begin(), chunk.end(), result.begin() + 5);
+ const std::streamsize len1 = f.sputn(chunk.data(), chunk_size1);
+ assert(len1 == chunk_size1);
+ // check that nothing in the original chunk was modified by sputn()
+ assert(std::strncmp(chunk.data(), data.substr(5, len1).c_str(), len1) == 0);
+
+ pos_type p1 = f.pubseekoff(offset1, std::ios_base::cur);
+ char c;
+ if (p1 < 0) {
+ p1 = f.pubseekoff(0, std::ios_base::beg);
+ assert(p1 == 0);
+ c = '^';
+ } else {
+ assert(p1 == 5 + len1 + offset1);
+ if (p1 > data_size) {
+ c = '_';
+ } else {
+ c = data[p1];
+ }
+ }
+
+ result[p1] = c;
+ assert(f.sputc(c) == c);
+
+ f.pubseekpos(std::ios_base::beg);
+ result[0] = 'A';
+ assert(f.sputc(toupper(data[0])) == 'A');
+
+ pos_type end_pos = f.pubseekoff(off_type(0), std::ios_base::end);
+ assert(f.sputc(toupper(data[data_size - 1])) == 'Z');
+ result[end_pos] = 'Z';
+
+ assert(f.pubseekpos(p1) == p1);
+ result[p1] = toupper(c);
+ assert(f.sputc(toupper(c)) == toupper(c));
+
+ pos_type new_pos = result_size - chunk_size2;
+ pos_type p2 = f.pubseekoff(new_pos, std::ios_base::beg);
+ assert(p2 == new_pos);
+ chunk = std::vector<char>(data.end() - chunk_size2, data.end());
+ std::copy(chunk.begin(), chunk.end(), result.begin() + p2);
+ const std::streamsize len2 = f.sputn(chunk.data(), chunk_size2);
+ assert(len2 == chunk_size2);
+ assert(std::strncmp(chunk.data(), data.substr(data_size - chunk_size2, chunk_size2).c_str(), len2) == 0);
+ f.close();
+ }
+ std::filebuf f;
+ assert(f.open("sputn_seekoff.dat", std::ios_base::in) != 0);
+ assert(f.is_open());
+ std::vector<char> check(result.size(), -1);
+ const std::size_t len = f.sgetn(check.data(), check.size());
+ assert(len == result.size());
+ for (size_t i = 0; i < len; ++i) {
+ assert(check[i] == result[i]);
+ }
+}
+
+int main(int, char**) {
+ sputn_seekoff(nullptr, 10, 22, -27, 1);
+ sputn_seekoff(nullptr, 10, 1, -27, 1);
+ sputn_seekoff(nullptr, 10, 10, 14, 12);
+ sputn_seekoff(nullptr, 10, 1, -2, 1);
+ sputn_seekoff(nullptr, 10, 10, -4, 12);
+ sputn_seekoff(nullptr, 10, 11, -12, 3);
+ sputn_seekoff(nullptr, 10, 7, 3, 8);
+ sputn_seekoff(nullptr, 10, 5, -5, 12);
+ sputn_seekoff(nullptr, 10, 1, 1, 1);
+ sputn_seekoff(nullptr, 10, 9, 0, 1);
+
+ // // the below are very long duration tests to cover many possible variations of I/O and hit different corner cases
+ // // may want to run them (for minutes) in case of some major refactoring or changes to implementation of stream class
+ // auto buffer_sizes = {7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 4096};
+ // for (std::size_t chunk_size1 = 1; chunk_size1 < 27; ++chunk_size1) {
+ // for (std::size_t chunk_size2 = 1; chunk_size2 < 27; ++chunk_size2) {
+ // for (off_type off1 = -27; off1 < 27; off1 += 1) {
+ // for (const auto& buf_sz : buffer_sizes) {
+ // sputn_seekoff(nullptr, buf_sz, chunk_size1, off1, chunk_size2);
+ // std::vector<char> buf;
+ // buf.resize(buf_sz);
+ // sputn_seekoff(buf.data(), buf_sz, chunk_size1, off1, chunk_size2);
+ // }
+ // }
+ // }
+ // }
+ return 0;
+}
More information about the libcxx-commits
mailing list