Skip to content

Commit

Permalink
QVariant: Use QMetaTypeInferface directly
Browse files Browse the repository at this point in the history 
There is no reason for QVariant to go through QMetaType when it can use
the QMetaTypeInterface directly. Without LTO, the QMetaType method calls
are opaque, and we therefore risk to lose optimizations.
Additionally, avoid constructing a QMetaType from a type id if we
already have the QMetaType.

Fixes: QTBUG-90673
Change-Id: I7069ff6aff70d5baecdf5cf5760014c3dda81784
Reviewed-by: Thiago Macieira <[email protected]>
  • Loading branch information
@Inkane
Inkane committed Jan 28, 2021
1 parent af53fb0 commit 40bfbe7
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 24 deletions.
64 changes: 47 additions & 17 deletions src/corelib/kernel/qvariant.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,24 +248,32 @@ static qreal qConvertToRealNumber(const QVariant::Private *d, bool *ok)
// the type of d has already been set, but other field are not set
static void customConstruct(QVariant::Private *d, const void *copy)
{
const QMetaType type = d->type();
const uint size = type.sizeOf();
if (!size) {
QtPrivate::QMetaTypeInterface *iface = d->typeInterface();
if (!(iface && iface->size)) {
*d = QVariant::Private();
return;
}

if (QVariant::Private::canUseInternalSpace(type)) {
type.construct(&d->data, copy);
if (QVariant::Private::canUseInternalSpace(iface)) {
// QVariant requires type to be copy and default constructible
Q_ASSERT(iface->copyCtr);
Q_ASSERT(iface->defaultCtr);
if (copy)
iface->copyCtr(iface, &d->data, copy);
else
iface->defaultCtr(iface, &d->data);
d->is_shared = false;
} else {
d->data.shared = QVariant::PrivateShared::create(type);
type.construct(d->data.shared->data(), copy);
d->data.shared = QVariant::PrivateShared::create(iface);
if (copy)
iface->copyCtr(iface, d->data.shared->data(), copy);
else
iface->defaultCtr(iface, d->data.shared->data());
d->is_shared = true;
}
// 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.
d->is_null = !copy || type == QMetaType::fromType<std::nullptr_t>();
d->is_null = !copy || QMetaType(iface) == QMetaType::fromType<std::nullptr_t>();
}

static void customClear(QVariant::Private *d)
Expand Down Expand Up @@ -471,11 +479,23 @@ static void customClear(QVariant::Private *d)
Constructs a variant private of type \a type, and initializes with \a copy if
\a copy is not \nullptr.
*/
*/
//### Qt 7: Remove in favor of QMetaType overload
void QVariant::create(int type, const void *copy)
{
d = Private(QMetaType(type));
create(QMetaType(type), copy);
}

/*!
\fn QVariant::create(int type, const void *copy)
\internal
\overload
*/
void QVariant::create(QMetaType type, const void *copy)
{
d = Private(type);
customConstruct(&d, copy);
}

Expand Down Expand Up @@ -510,9 +530,14 @@ QVariant::QVariant(const QVariant &p)
d.data.shared->ref.ref();
return;
}
QMetaType t = d.type();
if (t.isValid())
t.construct(&d, p.constData());
QtPrivate::QMetaTypeInterface *iface = d.typeInterface();
auto other = p.constData();
if (iface) {
if (other)
iface->copyCtr(iface, &d, other);
else
iface->defaultCtr(iface, &d);
}
}

/*!
Expand Down Expand Up @@ -995,9 +1020,14 @@ QVariant &QVariant::operator=(const QVariant &variant)
d = variant.d;
} else {
d = variant.d;
QMetaType t = d.type();
if (t.isValid())
t.construct(&d, variant.constData());
QtPrivate::QMetaTypeInterface *iface = d.typeInterface();
const void *other = variant.constData();
if (iface) {
if (other)
iface->copyCtr(iface, &d, other);
else
iface->defaultCtr(iface, &d);
}
}

return *this;
Expand Down Expand Up @@ -2021,7 +2051,7 @@ bool QVariant::convert(QMetaType targetType)
QVariant oldValue = *this;

clear();
create(targetType.id(), nullptr);
create(targetType, nullptr);
if (!oldValue.canConvert(targetType))
return false;

Expand Down
21 changes: 15 additions & 6 deletions src/corelib/kernel/qvariant.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,10 +426,11 @@ class Q_CORE_EXPORT QVariant
private:
inline PrivateShared() : ref(1) { }
public:
static PrivateShared *create(QMetaType type)
static PrivateShared *create(const QtPrivate::QMetaTypeInterface *type)
{
size_t size = type.sizeOf();
size_t align = type.alignOf();
Q_ASSERT(type);
size_t size = type->size;
size_t align = type->alignment;

size += sizeof(PrivateShared);
if (align > sizeof(PrivateShared)) {
Expand Down Expand Up @@ -463,10 +464,11 @@ class Q_CORE_EXPORT QVariant
static constexpr size_t MaxInternalSize = 3*sizeof(void *);
template<typename T>
static constexpr bool CanUseInternalSpace = (QTypeInfo<T>::isRelocatable && sizeof(T) <= MaxInternalSize && alignof(T) <= alignof(double));
static constexpr bool canUseInternalSpace(QMetaType type)
static constexpr bool canUseInternalSpace(QtPrivate::QMetaTypeInterface *type)
{
return type.flags() & QMetaType::RelocatableType &&
size_t(type.sizeOf()) <= MaxInternalSize && size_t(type.alignOf()) <= alignof(double);
Q_ASSERT(type);
return QMetaType::TypeFlags(type->flags) & QMetaType::RelocatableType &&
size_t(type->size) <= MaxInternalSize && size_t(type->alignment) <= alignof(double);
}

union
Expand Down Expand Up @@ -506,6 +508,12 @@ class Q_CORE_EXPORT QVariant
{
return QMetaType(reinterpret_cast<QtPrivate::QMetaTypeInterface *>(packedType << 2));
}

inline QtPrivate::QMetaTypeInterface * typeInterface() const
{
return reinterpret_cast<QtPrivate::QMetaTypeInterface *>(packedType << 2);
}

inline int typeId() const
{
return type().id();
Expand All @@ -531,6 +539,7 @@ class Q_CORE_EXPORT QVariant
protected:
Private d;
void create(int type, const void *copy);
void create(QMetaType type, const void *copy);
bool equals(const QVariant &other) const;
bool convert(int type, void *ptr) const;
bool view(int type, void *ptr);
Expand Down
2 changes: 1 addition & 1 deletion src/corelib/kernel/qvariant_p.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ inline void v_construct(QVariant::Private *x, const T &t)
new (&x->data) T(t);
x->is_shared = false;
} else {
x->data.shared = QVariant::PrivateShared::create(QMetaType::fromType<T>());
x->data.shared = QVariant::PrivateShared::create(QtPrivate::qMetaTypeInterfaceForType<T>());
new (x->data.shared->data()) T(t);
x->is_shared = true;
}
Expand Down

0 comments on commit 40bfbe7

Please sign in to comment.