-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[libc++] Optimize ofstream::write #123337
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
base: main
Are you sure you want to change the base?
Conversation
philnik777
commented
Jan 17, 2025
7b031d5
to
87acdbe
Compare
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) Changes
Full diff: https://github.com/llvm/llvm-project/pull/123337.diff 6 Files Affected:
diff --git a/libcxx/docs/ReleaseNotes/20.rst b/libcxx/docs/ReleaseNotes/20.rst
index 2736061544c531..50ab4aad48c24f 100644
--- a/libcxx/docs/ReleaseNotes/20.rst
+++ b/libcxx/docs/ReleaseNotes/20.rst
@@ -111,6 +111,9 @@ Improvements and New Features
std::errc::not_a_directory``, or use ``err.default_error_condition()`` to map to an ``error_condition``, and then test
its ``value()`` and ``category()``.
+- ``ofstream::write`` has been optimized to pass through large strings to system calls directly instead of copying them
+ in chunks into a buffer.
+
Deprecations and Removals
-------------------------
diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index f0e9425e0a53d9..491a94aca73974 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -232,6 +232,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;
@@ -304,6 +306,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 00000000000000..60606a9d67e2f5
--- /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 37ab176ea26a0f..9df0cffa1d441a 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@streambuf:* {{static assertion failed{{.*}}traits_type::char_type must be the same type as CharT}}
-// expected-error@fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error@fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error@fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error@fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error@fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error@fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error@fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error@fstream:* {{only virtual member functions can be marked 'override'}}
-// expected-error@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 f936d8db47af5b..007af7004dfc62 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@ios:* {{static assertion failed{{.*}}traits_type::char_type must be the same type as CharT}}
// expected-error-re@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 8008901802e91e..f6378e7998ee98 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');
|
87acdbe
to
11bc68b
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
11bc68b
to
6eeaa43
Compare
libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_not_open.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_seekoff.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_seekoff.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_seekoff.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/sputn_seekoff.pass.cpp
Outdated
Show resolved
Hide resolved
df49b3b
to
747a8e2
Compare
747a8e2
to
c5a0169
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with minor comments. This is a nice improvement!
// This isn't a required override by the standard, but most implementation override it, sine it allows for significantly | ||
// improved performance in some cases. All of this code is required to work, so this isn't an extension |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// This isn't a required override by the standard, but most implementation override it, sine it allows for significantly | |
// improved performance in some cases. All of this code is required to work, so this isn't an extension | |
// This isn't a required override by the standard, but most implementations override it, since it allows for significantly | |
// improved performance in some cases. All of this code is required to work, so this isn't a libc++ extension |
#include <codecvt> | ||
#include <fstream> | ||
#include <locale> | ||
#include <vector> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing include <cstring>
.
@@ -227,6 +227,8 @@ _LIBCPP_EXPORTED_FROM_ABI void* __filebuf_windows_native_handle(FILE* __file) no | |||
|
|||
template <class _CharT, class _Traits> | |||
class basic_filebuf : public basic_streambuf<_CharT, _Traits> { | |||
using __base _LIBCPP_NODEBUG = basic_streambuf<_CharT, _Traits>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use basic_streambuf
directly and avoid introducing this typedef which is used only once.