From 353d22756fa995e2f50e1af92f49728532cd29ff Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 20 Dec 2024 22:00:32 +0100 Subject: [PATCH] Fix a performance regression in QDataStream Commit e176dd78fd2f253eb2625585b2bd90b5713e5984 replaced a `new char[n]` with a std::make_unique(n), probably on this author's insistence. But the two are not equivalent: make_unique() value-initializes, even arrays, even of built-in type, which means that each buffer resize writes each byte twice: first nulling out the whole buffer as part of value-initialization, then in the memcpy() and any following read()s. For buffers of several MiB, or even GiB in size, this is very costly. Fix by adding and using a backport of C++20 make_unique_for_overwrite(), which performs the equivalent of the old code (ie. default-, not value-initialization). Also add q20::is_(un)bounded_array, which are needed for the implementation of q20::make_unique_for_overwrite(). Amends e176dd78fd2f253eb2625585b2bd90b5713e5984. Pick-to: 6.8 6.5 Change-Id: I8865c7369e522ec475df122e3d00d6aba3b24561 Reviewed-by: Thiago Macieira (cherry picked from commit 1a9f8cc0df33195df959cee2e355dde4cbacd754) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/global/q20memory.h | 28 ++++++++++++++++++++++- src/corelib/global/q20type_traits.h | 17 ++++++++++++++ src/corelib/serialization/qdatastream.cpp | 4 +++- 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/corelib/global/q20memory.h b/src/corelib/global/q20memory.h index d11725ce5ce..6dd4bf5afae 100644 --- a/src/corelib/global/q20memory.h +++ b/src/corelib/global/q20memory.h @@ -9,7 +9,7 @@ #include -#include +#include #include // @@ -46,6 +46,32 @@ T *construct_at(T *ptr, Args && ... args) #endif // __cpp_lib_constexpr_dynamic_alloc } // namespace q20 +// like std::make_unique_for_overwrite (excl. C++23-added constexpr) +namespace q20 { +#ifdef __cpp_lib_smart_ptr_for_overwrite +using std::make_unique_for_overwrite; +#else +// https://eel.is/c++draft/unique.ptr.create#6 +template +std::enable_if_t, std::unique_ptr> +make_unique_for_overwrite() +// https://eel.is/c++draft/unique.ptr.create#7 +{ return std::unique_ptr(new T); } + +// https://eel.is/c++draft/unique.ptr.create#8 +template +std::enable_if_t, std::unique_ptr> +make_unique_for_overwrite(std::size_t n) +// https://eel.is/c++draft/unique.ptr.create#9 +{ return std::unique_ptr(new std::remove_extent_t[n]); } + +// https://eel.is/c++draft/unique.ptr.create#10 +template +std::enable_if_t> +make_unique_for_overwrite(Args&&...) = delete; + +#endif // __cpp_lib_smart_ptr_for_overwrite +} // namespace q20 namespace q20 { // like std::to_address diff --git a/src/corelib/global/q20type_traits.h b/src/corelib/global/q20type_traits.h index 63a453daca8..97614fbab6a 100644 --- a/src/corelib/global/q20type_traits.h +++ b/src/corelib/global/q20type_traits.h @@ -27,6 +27,23 @@ QT_BEGIN_NAMESPACE +namespace q20 { +// like std::is_(un)bounded_array +#ifdef __cpp_lib_bounded_array_traits +using std::is_bounded_array; +using std::is_bounded_array_v; +using std::is_unbounded_array; +using std::is_unbounded_array_v; +#else +template struct is_bounded_array : std::false_type {}; +template struct is_bounded_array : std::true_type {}; +template struct is_unbounded_array : std::false_type {}; +template struct is_unbounded_array : std::true_type {}; +template constexpr inline bool is_bounded_array_v = q20::is_bounded_array::value; +template constexpr inline bool is_unbounded_array_v = q20::is_unbounded_array::value; +#endif +} + namespace q20 { // like std::is_constant_evaluated #ifdef __cpp_lib_is_constant_evaluated diff --git a/src/corelib/serialization/qdatastream.cpp b/src/corelib/serialization/qdatastream.cpp index 9ba93b654ae..0b9c16ff0af 100644 --- a/src/corelib/serialization/qdatastream.cpp +++ b/src/corelib/serialization/qdatastream.cpp @@ -13,6 +13,8 @@ #include #include "qendian.h" +#include + QT_BEGIN_NAMESPACE constexpr quint32 QDataStream::NullCode; @@ -1095,7 +1097,7 @@ QDataStream &QDataStream::readBytes(char *&s, qint64 &l) do { qsizetype blockSize = qMin(step, len - allocated); const qsizetype n = allocated + blockSize + 1; - if (const auto prevBuf = std::exchange(curBuf, std::make_unique(n))) + if (const auto prevBuf = std::exchange(curBuf, q20::make_unique_for_overwrite(n))) memcpy(curBuf.get(), prevBuf.get(), allocated); if (readBlock(curBuf.get() + allocated, blockSize) != blockSize) return *this;