diff options
author | Fabian Kosmale <fabian.kosmale@qt.io> | 2022-09-18 15:27:38 +0200 |
---|---|---|
committer | Marc Mutz <marc.mutz@qt.io> | 2023-06-02 22:31:36 +0200 |
commit | 79ae79d05c65019233cf9ae9e77ef59d90e75ac2 (patch) | |
tree | 417e8dcfed1af2c78a9de0daaa8e059afce2590c /src | |
parent | 581c4bcb62a9d3cbb4c33df3f0f7a0a965225e74 (diff) |
QVariant::fromValue: Add rvalue optimization
When passing an rvalue-reference to QVariant, there is no reason to make
a copy if the type is moveable. Moreover, we know that the pointer which
we construct from the object passed to fromValue non-null. We make use
of both facts by parametrizing custom_construct on
non-nullness and availability of a move-ctor, and then dispatching to
the suitable template.
We need to keep the const T& overload, as otherwise code which
explicitly specializes fromValue and passes a const lvalue to it would
stop to compile.
[ChangeLog][QtCore][QVariant] Added fromValue() overload taking rvalues.
Change-Id: I44fb757d516ef364fe7967bc103b3f98278b4919
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Diffstat (limited to 'src')
-rw-r--r-- | src/corelib/kernel/qmetatype_p.h | 9 | ||||
-rw-r--r-- | src/corelib/kernel/qvariant.cpp | 74 | ||||
-rw-r--r-- | src/corelib/kernel/qvariant.h | 39 |
3 files changed, 117 insertions, 5 deletions
diff --git a/src/corelib/kernel/qmetatype_p.h b/src/corelib/kernel/qmetatype_p.h index e649394832..71a225d5e6 100644 --- a/src/corelib/kernel/qmetatype_p.h +++ b/src/corelib/kernel/qmetatype_p.h @@ -178,6 +178,15 @@ inline void copyConstruct(const QtPrivate::QMetaTypeInterface *iface, void *wher memcpy(where, copy, iface->size); } +inline void moveConstruct(const QtPrivate::QMetaTypeInterface *iface, void *where, void *copy) +{ + Q_ASSERT(isMoveConstructible(iface)); + if (iface->moveCtr) + iface->moveCtr(iface, where, copy); + else + memcpy(where, copy, iface->size); +} + inline void construct(const QtPrivate::QMetaTypeInterface *iface, void *where, const void *copy) { if (copy) diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp index ebb99d3180..8e2457350c 100644 --- a/src/corelib/kernel/qvariant.cpp +++ b/src/corelib/kernel/qvariant.cpp @@ -47,6 +47,8 @@ #include "qline.h" #endif +#include <memory> + #include <cmath> #include <float.h> #include <cstring> @@ -231,9 +233,38 @@ static bool isValidMetaTypeForVariant(const QtPrivate::QMetaTypeInterface *iface return true; } +enum CustomConstructMoveOptions { + UseCopy, // custom construct uses the copy ctor unconditionally + // future option: TryMove: uses move ctor if available, else copy ctor + ForceMove, // custom construct use the move ctor (which must exist) +}; + +enum CustomConstructNullabilityOption { + MaybeNull, // copy might be null, might be non-null + NonNull, // copy is guarantueed to be non-null + // future option: AlwaysNull? +}; + + +template <typename F> static QVariant::PrivateShared * +customConstructShared(size_t size, size_t align, F &&construct) +{ + struct Deleter { + void operator()(QVariant::PrivateShared *p) const + { QVariant::PrivateShared::free(p); } + }; + + // this is exception-safe + std::unique_ptr<QVariant::PrivateShared, Deleter> ptr; + ptr.reset(QVariant::PrivateShared::create(size, align)); + construct(ptr->data()); + return ptr.release(); +} + // the type of d has already been set, but other field are not set +template <CustomConstructMoveOptions moveOption = UseCopy, CustomConstructNullabilityOption nullability = MaybeNull> static void customConstruct(const QtPrivate::QMetaTypeInterface *iface, QVariant::Private *d, - const void *copy) + std::conditional_t<moveOption == ForceMove, void *, const void *> copy) { using namespace QtMetaTypePrivate; Q_ASSERT(iface); @@ -242,6 +273,10 @@ static void customConstruct(const QtPrivate::QMetaTypeInterface *iface, QVariant Q_ASSERT(isCopyConstructible(iface)); Q_ASSERT(isDestructible(iface)); Q_ASSERT(copy || isDefaultConstructible(iface)); + if constexpr (moveOption == ForceMove) + Q_ASSERT(isMoveConstructible(iface)); + if constexpr (nullability == NonNull) + Q_ASSUME(copy != nullptr); // need to check for nullptr_t here, as this can get called by fromValue(nullptr). fromValue() uses // std::addressof(value) which in this case returns the address of the nullptr object. @@ -251,11 +286,17 @@ static void customConstruct(const QtPrivate::QMetaTypeInterface *iface, QVariant if (QVariant::Private::canUseInternalSpace(iface)) { d->is_shared = false; if (!copy && !iface->defaultCtr) - return; // default constructor and it's OK to build in 0-filled storage, which we've already done - construct(iface, d->data.data, copy); + return; // trivial default constructor and it's OK to build in 0-filled storage, which we've already done + if constexpr (moveOption == ForceMove && nullability == NonNull) + moveConstruct(iface, d->data.data, copy); + else + construct(iface, d->data.data, copy); } else { d->data.shared = customConstructShared(iface->size, iface->alignment, [=](void *where) { - construct(iface, where, copy); + if constexpr (moveOption == ForceMove && nullability == NonNull) + moveConstruct(iface, where, copy); + else + construct(iface, where, copy); }); d->is_shared = true; } @@ -1064,7 +1105,8 @@ void QVariant::detach() Q_ASSERT(isValidMetaTypeForVariant(d.typeInterface(), constData())); Private dd(d.typeInterface()); - customConstruct(d.typeInterface(), &dd, constData()); + // null variant is never shared; anything else is NonNull + customConstruct<UseCopy, NonNull>(d.typeInterface(), &dd, constData()); if (!d.data.shared->ref.deref()) customClear(&d); d.data.shared = dd.data.shared; @@ -2524,6 +2566,22 @@ QDebug QVariant::qdebugHelper(QDebug dbg) const return dbg; } +QVariant QVariant::moveConstruct(QMetaType type, void *data) +{ + QVariant var; + var.d = QVariant::Private(type.d_ptr); + customConstruct<ForceMove, NonNull>(type.d_ptr, &var.d, data); + return var; +} + +QVariant QVariant::copyConstruct(QMetaType type, const void *data) +{ + QVariant var; + var.d = QVariant::Private(type.d_ptr); + customConstruct<UseCopy, NonNull>(type.d_ptr, &var.d, data); + return var; +} + #if QT_DEPRECATED_SINCE(6, 0) QT_WARNING_PUSH QT_WARNING_DISABLE_DEPRECATED @@ -2644,6 +2702,12 @@ QT_WARNING_POP \sa setValue(), value() */ +/*! \fn template<typename T> static QVariant QVariant::fromValue(T &&value) + + \since 6.6 + \overload +*/ + /*! \fn template<typename... Types> QVariant QVariant::fromStdVariant(const std::variant<Types...> &value) \since 5.11 diff --git a/src/corelib/kernel/qvariant.h b/src/corelib/kernel/qvariant.h index b1865e442b..582f135fab 100644 --- a/src/corelib/kernel/qvariant.h +++ b/src/corelib/kernel/qvariant.h @@ -71,6 +71,9 @@ class Q_CORE_EXPORT QVariant >, bool>; + template <typename T> + using if_rvalue = std::enable_if_t<!std::is_reference_v<T>, bool>; + struct CborValueStandIn { qint64 n; void *c; int t; }; public: struct PrivateShared @@ -516,6 +519,39 @@ public: return t; } + template<typename T, if_rvalue<T> = true> +#ifndef Q_QDOC + /* needs is_copy_constructible for variants semantics, is_move_constructible so that moveConstruct works + (but copy_constructible implies move_constructble, so don't bother checking) + */ + static inline auto fromValue(T &&value) + noexcept(std::is_nothrow_copy_constructible_v<T> && Private::CanUseInternalSpace<T>) + -> std::enable_if_t<std::conjunction_v<std::is_copy_constructible<T>, + std::is_destructible<T>>, QVariant> +#else + static inline QVariant fromValue(T &&value) +#endif + { + // handle special cases + using Type = std::remove_cv_t<T>; + if constexpr (std::is_null_pointer_v<Type>) + return QVariant(QMetaType::fromType<std::nullptr_t>()); + else if constexpr (std::is_same_v<Type, QVariant>) + return std::forward<T>(value); + else if constexpr (std::is_same_v<Type, std::monostate>) + return QVariant(); + QMetaType mt = QMetaType::fromType<Type>(); + mt.registerType(); // we want the type stored in QVariant to always be registered + // T is a forwarding reference, so if T satifies the enable-ifery, + // we get this overload even if T is an lvalue reference and thus must check here + // Moreover, we only try to move if the type is actually moveable and not if T is const + // as in const int i; QVariant::fromValue(std::move(i)); + if constexpr (std::conjunction_v<std::is_move_constructible<Type>, std::negation<std::is_const<T>>>) + return moveConstruct(QMetaType::fromType<Type>(), std::addressof(value)); + else + return copyConstruct(mt, std::addressof(value)); + } + template<typename T> #ifndef Q_QDOC static inline auto fromValue(const T &value) @@ -594,6 +630,9 @@ private: Q_MK_GET(const &&) #undef Q_MK_GET + static QVariant moveConstruct(QMetaType type, void *data); + static QVariant copyConstruct(QMetaType type, const void *data); + template<typename T> friend inline T qvariant_cast(const QVariant &); protected: |