From 705ea62f9e08730793adc7f6cec753c88e66ac4e Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sat, 27 Nov 2021 10:29:35 -0800 Subject: [PATCH] QJsonValueConstRef: prepare for Qt 7 Instead of storing a pointer to the QJsonArray or QJsonObject (like the Qt 4 & pre-5.15 versions did), do like QCborValueConstRef and store the pointer to the QCborContainerPrivate. Unlike QCborValueRef, we must keep the is_object bit because of API behavior that assigning an Undefined to an object reference deletes it from the container. I've chosen to use size_t instead of qsizetype for this because then we don't lose any bits of the index. Therefore, the index in the case of objects is stored as pair count (like before), different from QCborValueRef which stores the actual index in the QCborContainerPrivate. It's the LSB (on little-endian architectures) so the calculation of 2 * index + 1 is the actual value stored in memory or in the register. Unfortunately, right now, both Clang and GCC don't realize this and generate unnecessary instructions. Clang: 0000000000000000 : 0: mov %rsi,%rax 3: shr %rax 6: mov %rsi,%rcx 9: or $0x1,%rcx d: test $0x1,%sil 11: cmove %rax,%rcx [GCC code is identical, except it uses an AND instead of TEST] That OR at offset 9 is a no-op because it sets a bit that is already set in those conditions. At least they don't do unnecessary shifts. Change-Id: I89446ea06b5742efb194fffd16bb7aadb6a9b341 Reviewed-by: Sona Kurazyan --- src/corelib/serialization/qjson_p.h | 10 ++++- src/corelib/serialization/qjsonarray.h | 30 +++++++------ src/corelib/serialization/qjsonobject.cpp | 2 +- src/corelib/serialization/qjsonobject.h | 42 +++++++++-------- src/corelib/serialization/qjsonvalue.cpp | 16 +++++++ src/corelib/serialization/qjsonvalue.h | 55 ++++++++++++++++++----- 6 files changed, 110 insertions(+), 45 deletions(-) diff --git a/src/corelib/serialization/qjson_p.h b/src/corelib/serialization/qjson_p.h index 37b556b9d01..959058ab2ab 100644 --- a/src/corelib/serialization/qjson_p.h +++ b/src/corelib/serialization/qjson_p.h @@ -56,8 +56,10 @@ #include #include -#include -#include +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) && !defined(QT_BOOTSTRAPPED) +# include +# include +#endif QT_BEGIN_NAMESPACE @@ -209,7 +211,11 @@ public: static QCborContainerPrivate *container(const QCborValue &v) { return v.container; } static const QCborContainerPrivate *container(QJsonValueConstRef r) noexcept { +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) && !defined(QT_BOOTSTRAPPED) return (r.is_object ? r.o->o : r.a->a).data(); +#else + return r.d; +#endif } static QCborContainerPrivate *container(QJsonValueRef r) noexcept { diff --git a/src/corelib/serialization/qjsonarray.h b/src/corelib/serialization/qjsonarray.h index d8a7e2e12b7..20109dfbf31 100644 --- a/src/corelib/serialization/qjsonarray.h +++ b/src/corelib/serialization/qjsonarray.h @@ -120,8 +120,7 @@ public: constexpr iterator(const iterator &other) = default; iterator &operator=(const iterator &other) { - item.a = other.item.a; - item.index = other.item.index; + item.rebind(other.item); return *this; } @@ -131,21 +130,21 @@ public: inline QJsonValueRef operator[](qsizetype j) const { return *(*this + j); } inline bool operator==(const iterator &o) const - { return item.a == o.item.a && item.index == o.item.index; } + { return item.d == o.item.d && item.index == o.item.index; } inline bool operator!=(const iterator &o) const { return !(*this == o); } inline bool operator<(const iterator &other) const - { Q_ASSERT(item.a == other.item.a); return item.index < other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index < other.item.index; } inline bool operator<=(const iterator &other) const - { Q_ASSERT(item.a == other.item.a); return item.index <= other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index <= other.item.index; } inline bool operator>(const iterator &other) const { return !(*this <= other); } inline bool operator>=(const iterator &other) const { return !(*this < other); } inline bool operator==(const const_iterator &o) const - { return item.a == o.item.a && item.index == o.item.index; } + { return item.d == o.item.d && item.index == o.item.index; } inline bool operator!=(const const_iterator &o) const { return !(*this == o); } inline bool operator<(const const_iterator &other) const - { Q_ASSERT(item.a == other.item.a); return item.index < other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index < other.item.index; } inline bool operator<=(const const_iterator &other) const - { Q_ASSERT(item.a == other.item.a); return item.index <= other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index <= other.item.index; } inline bool operator>(const const_iterator &other) const { return !(*this <= other); } inline bool operator>=(const const_iterator &other) const { return !(*this < other); } inline iterator &operator++() { ++item.index; return *this; } @@ -180,8 +179,7 @@ public: constexpr const_iterator(const const_iterator &other) = default; const_iterator &operator=(const const_iterator &other) { - item.a = other.item.a; - item.index = other.item.index; + item.rebind(other.item); return *this; } @@ -190,12 +188,12 @@ public: inline QJsonValueConstRef operator[](qsizetype j) const { return *(*this + j); } inline bool operator==(const const_iterator &o) const - { return item.a == o.item.a && item.index == o.item.index; } + { return item.d == o.item.d && item.index == o.item.index; } inline bool operator!=(const const_iterator &o) const { return !(*this == o); } inline bool operator<(const const_iterator &other) const - { Q_ASSERT(item.a == other.item.a); return item.index < other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index < other.item.index; } inline bool operator<=(const const_iterator &other) const - { Q_ASSERT(item.a == other.item.a); return item.index <= other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index <= other.item.index; } inline bool operator>(const const_iterator &other) const { return !(*this <= other); } inline bool operator>=(const const_iterator &other) const { return !(*this < other); } inline const_iterator &operator++() { ++item.index; return *this; } @@ -271,6 +269,12 @@ private: Q_DECLARE_SHARED(QJsonArray) +#if QT_VERSION >= QT_VERSION_CHECK(7, 0, 0) || defined(QT_BOOTSTRAPPED) +inline QJsonValueConstRef::QJsonValueConstRef(QJsonArray *a, qsizetype idx) + : d(a->a.data()), is_object(false), index(idx) +{} +#endif + Q_CORE_EXPORT size_t qHash(const QJsonArray &array, size_t seed = 0); #if !defined(QT_NO_DEBUG_STREAM) && !defined(QT_JSON_READONLY) diff --git a/src/corelib/serialization/qjsonobject.cpp b/src/corelib/serialization/qjsonobject.cpp index 2497f42a1a1..a8cfc4372ca 100644 --- a/src/corelib/serialization/qjsonobject.cpp +++ b/src/corelib/serialization/qjsonobject.cpp @@ -1419,6 +1419,7 @@ bool QJsonObject::detach(qsizetype reserve) return o; } +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) && !defined(QT_BOOTSTRAPPED) /*! \internal */ @@ -1428,7 +1429,6 @@ QString QJsonObject::keyAt(qsizetype i) const return o->stringAt(i * 2); } -#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) && !defined(QT_BOOTSTRAPPED) /*! \internal */ diff --git a/src/corelib/serialization/qjsonobject.h b/src/corelib/serialization/qjsonobject.h index 1389f7b4722..573d526dff7 100644 --- a/src/corelib/serialization/qjsonobject.h +++ b/src/corelib/serialization/qjsonobject.h @@ -136,12 +136,11 @@ public: constexpr iterator(const iterator &other) = default; iterator &operator=(const iterator &other) { - item.o = other.item.o; - item.index = other.item.index; + item.rebind(other.item); return *this; } - inline QString key() const { return item.o->keyAt(item.index); } + inline QString key() const { return item.objectKey(); } inline QJsonValueRef value() const { return item; } inline QJsonValueRef operator*() const { return item; } inline const QJsonValueConstRef *operator->() const { return &item; } @@ -149,12 +148,12 @@ public: inline QJsonValueRef operator[](qsizetype j) const { return *(*this + j); } inline bool operator==(const iterator &other) const - { return item.o == other.item.o && item.index == other.item.index; } + { return item.d == other.item.d && item.index == other.item.index; } inline bool operator!=(const iterator &other) const { return !(*this == other); } bool operator<(const iterator& other) const - { Q_ASSERT(item.o == other.item.o); return item.index < other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index < other.item.index; } bool operator<=(const iterator& other) const - { Q_ASSERT(item.o == other.item.o); return item.index <= other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index <= other.item.index; } bool operator>(const iterator& other) const { return !(*this <= other); } bool operator>=(const iterator& other) const { return !(*this < other); } @@ -170,12 +169,12 @@ public: public: inline bool operator==(const const_iterator &other) const - { return item.o == other.item.o && item.index == other.item.index; } + { return item.d == other.item.d && item.index == other.item.index; } inline bool operator!=(const const_iterator &other) const { return !(*this == other); } bool operator<(const const_iterator& other) const - { Q_ASSERT(item.o == other.item.o); return item.index < other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index < other.item.index; } bool operator<=(const const_iterator& other) const - { Q_ASSERT(item.o == other.item.o); return item.index <= other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index <= other.item.index; } bool operator>(const const_iterator& other) const { return !(*this <= other); } bool operator>=(const const_iterator& other) const { return !(*this < other); } }; @@ -202,24 +201,23 @@ public: constexpr const_iterator(const const_iterator &other) = default; const_iterator &operator=(const const_iterator &other) { - item.o = other.item.o; - item.index = other.item.index; + item.rebind(other.item); return *this; } - inline QString key() const { return item.o->keyAt(item.index); } + inline QString key() const { return item.objectKey(); } inline QJsonValueConstRef value() const { return item; } inline const QJsonValueConstRef operator*() const { return item; } inline const QJsonValueConstRef *operator->() const { return &item; } inline QJsonValueConstRef operator[](qsizetype j) const { return *(*this + j); } inline bool operator==(const const_iterator &other) const - { return item.o == other.item.o && item.index == other.item.index; } + { return item.d == other.item.d && item.index == other.item.index; } inline bool operator!=(const const_iterator &other) const { return !(*this == other); } bool operator<(const const_iterator& other) const - { Q_ASSERT(item.o == other.item.o); return item.index < other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index < other.item.index; } bool operator<=(const const_iterator& other) const - { Q_ASSERT(item.o == other.item.o); return item.index <= other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index <= other.item.index; } bool operator>(const const_iterator& other) const { return !(*this <= other); } bool operator>=(const const_iterator& other) const { return !(*this < other); } @@ -234,12 +232,12 @@ public: qsizetype operator-(const_iterator j) const { return item.index - j.item.index; } inline bool operator==(const iterator &other) const - { return item.o == other.item.o && item.index == other.item.index; } + { return item.d == other.item.d && item.index == other.item.index; } inline bool operator!=(const iterator &other) const { return !(*this == other); } bool operator<(const iterator& other) const - { Q_ASSERT(item.o == other.item.o); return item.index < other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index < other.item.index; } bool operator<=(const iterator& other) const - { Q_ASSERT(item.o == other.item.o); return item.index <= other.item.index; } + { Q_ASSERT(item.d == other.item.d); return item.index <= other.item.index; } bool operator>(const iterator& other) const { return !(*this <= other); } bool operator>=(const iterator& other) const { return !(*this < other); } }; @@ -300,8 +298,8 @@ private: template const_iterator constFindImpl(T key) const; template iterator insertImpl(T key, const QJsonValue &value); - QString keyAt(qsizetype i) const; #if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) && !defined(QT_BOOTSTRAPPED) + QString keyAt(qsizetype i) const; QJsonValue valueAt(qsizetype i) const; void setValueAt(qsizetype i, const QJsonValue &val); #endif @@ -313,6 +311,12 @@ private: Q_DECLARE_SHARED(QJsonObject) +#if QT_VERSION >= QT_VERSION_CHECK(7, 0, 0) || defined(QT_BOOTSTRAPPED) +inline QJsonValueConstRef::QJsonValueConstRef(QJsonObject *o, qsizetype idx) + : d(o->o.data()), is_object(true), index(idx) +{} +#endif + Q_CORE_EXPORT size_t qHash(const QJsonObject &object, size_t seed = 0); #if !defined(QT_NO_DEBUG_STREAM) && !defined(QT_JSON_READONLY) diff --git a/src/corelib/serialization/qjsonvalue.cpp b/src/corelib/serialization/qjsonvalue.cpp index 9fecbbb6266..37ead9648eb 100644 --- a/src/corelib/serialization/qjsonvalue.cpp +++ b/src/corelib/serialization/qjsonvalue.cpp @@ -921,6 +921,7 @@ bool QJsonValue::operator!=(const QJsonValue &other) const void QJsonValueRef::detach() { +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) && !defined(QT_BOOTSTRAPPED) QCborContainerPrivate *d = QJsonPrivate::Value::container(*this); d = QCborContainerPrivate::detach(d, d->elements.size()); @@ -928,6 +929,9 @@ void QJsonValueRef::detach() o->o.reset(d); else a->a.reset(d); +#else + d = QCborContainerPrivate::detach(d, d->elements.size()); +#endif } static QJsonValueRef &assignToRef(QJsonValueRef &ref, const QCborValue &value, bool is_object) @@ -1036,6 +1040,18 @@ QJsonValue QJsonValueConstRef::concrete(QJsonValueConstRef self) noexcept return QJsonPrivate::Value::fromTrustedCbor(d->valueAt(index)); } +QString QJsonValueConstRef::objectKey(QJsonValueConstRef self) +{ + Q_ASSERT(self.is_object); + Q_ASSUME(self.is_object); + const QCborContainerPrivate *d = QJsonPrivate::Value::container(self); + qsizetype index = QJsonPrivate::Value::indexHelper(self); + + Q_ASSERT(d); + Q_ASSERT(index < d->elements.size()); + return d->stringAt(index - 1); +} + #if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) && !defined(QT_BOOTSTRAPPED) QVariant QJsonValueRef::toVariant() const { diff --git a/src/corelib/serialization/qjsonvalue.h b/src/corelib/serialization/qjsonvalue.h index f50bb08017d..06dfb55e19e 100644 --- a/src/corelib/serialization/qjsonvalue.h +++ b/src/corelib/serialization/qjsonvalue.h @@ -190,11 +190,6 @@ public: inline bool operator!=(const QJsonValue &other) const { return concrete(*this) != other; } protected: - QJsonValueConstRef(QJsonArray *array, qsizetype idx) - : a(array), is_object(false), index(static_cast(idx)) {} - QJsonValueConstRef(QJsonObject *object, qsizetype idx) - : o(object), is_object(true), index(static_cast(idx)) {} - Q_CORE_EXPORT static QJsonValue::Type concreteType(QJsonValueConstRef self) noexcept Q_DECL_PURE_FUNCTION; Q_CORE_EXPORT static bool @@ -206,12 +201,48 @@ protected: Q_CORE_EXPORT static QString concreteString(QJsonValueConstRef self, const QString &defaultValue); Q_CORE_EXPORT static QJsonValue concrete(QJsonValueConstRef self) noexcept; + // for iterators + Q_CORE_EXPORT static QString objectKey(QJsonValueConstRef self); + QString objectKey() const { return objectKey(*this); } + +#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0) && !defined(QT_BOOTSTRAPPED) + QJsonValueConstRef(QJsonArray *array, qsizetype idx) + : a(array), is_object(false), index(static_cast(idx)) {} + QJsonValueConstRef(QJsonObject *object, qsizetype idx) + : o(object), is_object(true), index(static_cast(idx)) {} + + void rebind(QJsonValueConstRef other) + { + Q_ASSERT(is_object == other.is_object); + if (is_object) + o = other.o; + else + a = other.a; + index = other.index; + } + union { QJsonArray *a; QJsonObject *o; + void *d; }; quint64 is_object : 1; quint64 index : 63; +#else + // implemented in qjsonarray.h & qjsonobject.h, to get their d + QJsonValueConstRef(QJsonArray *array, qsizetype idx); + QJsonValueConstRef(QJsonObject *object, qsizetype idx); + + void rebind(QJsonValueConstRef other) + { + d = other.d; + index = other.index; + } + + QCborContainerPrivate *d = nullptr; + size_t is_object : 1; + size_t index : std::numeric_limits::digits - 1; +#endif friend class QJsonArray; friend class QJsonObject; @@ -221,11 +252,6 @@ protected: class Q_CORE_EXPORT QJsonValueRef : public QJsonValueConstRef { public: - QJsonValueRef(QJsonArray *array, qsizetype idx) - : QJsonValueConstRef(array, idx) {} - QJsonValueRef(QJsonObject *object, qsizetype idx) - : QJsonValueConstRef(object, idx) {} - QJsonValueRef(const QJsonValueRef &) = default; QJsonValueRef &operator = (const QJsonValue &val); QJsonValueRef &operator = (const QJsonValueRef &val); @@ -234,6 +260,11 @@ public: // retained for binary compatibility (due to the Q_CORE_EXPORT) because at // least one compiler emits and exports all inlines in an exported class + QJsonValueRef(QJsonArray *array, qsizetype idx) + : QJsonValueConstRef(array, idx) {} + QJsonValueRef(QJsonObject *object, qsizetype idx) + : QJsonValueConstRef(object, idx) {} + operator QJsonValue() const { return toValue(); } QVariant toVariant() const; @@ -265,8 +296,12 @@ private: QJsonValue toValue() const; #else private: + using QJsonValueConstRef::QJsonValueConstRef; #endif // < Qt 7 + void detach(); + friend class QJsonArray; + friend class QJsonObject; }; inline QJsonValue QCborValueConstRef::toJsonValue() const