Skip to content

Commit

Permalink
Fix a performance regression in QDataStream
Browse files Browse the repository at this point in the history
Commit e176dd7 replaced a `new
char[n]` with a std::make_unique<char[]>(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 e176dd7.

Pick-to: 6.8 6.5
Change-Id: I8865c7369e522ec475df122e3d00d6aba3b24561
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 1a9f8cc)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
  • Loading branch information
marcmutz authored and Qt Cherry-pick Bot committed Dec 24, 2024
1 parent aff0827 commit 353d227
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 2 deletions.
28 changes: 27 additions & 1 deletion src/corelib/global/q20memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

#include <QtCore/q17memory.h>

#include <type_traits>
#include <QtCore/q20type_traits.h>
#include <utility>

//
Expand Down Expand Up @@ -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 <typename T>
std::enable_if_t<!std::is_array_v<T>, std::unique_ptr<T>>
make_unique_for_overwrite()
// https://eel.is/c++draft/unique.ptr.create#7
{ return std::unique_ptr<T>(new T); }

// https://eel.is/c++draft/unique.ptr.create#8
template <typename T>
std::enable_if_t<q20::is_unbounded_array_v<T>, std::unique_ptr<T>>
make_unique_for_overwrite(std::size_t n)
// https://eel.is/c++draft/unique.ptr.create#9
{ return std::unique_ptr<T>(new std::remove_extent_t<T>[n]); }

// https://eel.is/c++draft/unique.ptr.create#10
template <typename T, typename...Args>
std::enable_if_t<q20::is_bounded_array_v<T>>
make_unique_for_overwrite(Args&&...) = delete;

#endif // __cpp_lib_smart_ptr_for_overwrite
} // namespace q20

namespace q20 {
// like std::to_address
Expand Down
17 changes: 17 additions & 0 deletions src/corelib/global/q20type_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename T> struct is_bounded_array : std::false_type {};
template <typename T, std::size_t N> struct is_bounded_array<T[N]> : std::true_type {};
template <typename T> struct is_unbounded_array : std::false_type {};
template <typename T> struct is_unbounded_array<T[]> : std::true_type {};
template <typename T> constexpr inline bool is_bounded_array_v = q20::is_bounded_array<T>::value;
template <typename T> constexpr inline bool is_unbounded_array_v = q20::is_unbounded_array<T>::value;
#endif
}

namespace q20 {
// like std::is_constant_evaluated
#ifdef __cpp_lib_is_constant_evaluated
Expand Down
4 changes: 3 additions & 1 deletion src/corelib/serialization/qdatastream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#include <stdlib.h>
#include "qendian.h"

#include <QtCore/q20memory.h>

QT_BEGIN_NAMESPACE

constexpr quint32 QDataStream::NullCode;
Expand Down Expand Up @@ -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<char[]>(n)))
if (const auto prevBuf = std::exchange(curBuf, q20::make_unique_for_overwrite<char[]>(n)))
memcpy(curBuf.get(), prevBuf.get(), allocated);
if (readBlock(curBuf.get() + allocated, blockSize) != blockSize)
return *this;
Expand Down

0 comments on commit 353d227

Please sign in to comment.