IPC: Remove the "small" object optimization in QNativeIpcKey

The original design was supposed to be small, at a single pointer, but
that never made it through, with the "QString key" member. So the
anonymous union for typeAndFlags overlapping with the extension pointer
was just unnecessary headache.

So separate the two. This means QNativeIpcKey's size increases from 4
pointers to 40 bytes on 64-bit systems and to 24 bytes on 32-bit systems
(so we have 6 unused bytes on both architectures).

Fixes: QTBUG-116821
Pick-to: 6.6.0
Change-Id: I512648fd617741199e67fffd1782b7d5ba5ddd12
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit eb5c5a76cf369dd5e40289c1013f043c7e82ff2e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Thiago Macieira 2023-09-07 13:28:58 -07:00 committed by Qt Cherry-pick Bot
parent 608b08f4d5
commit ef5f2322a7
3 changed files with 45 additions and 84 deletions

View File

@ -375,8 +375,7 @@ QNativeIpcKey::Type QNativeIpcKey::defaultTypeForOs_internal() noexcept
*/ */
void QNativeIpcKey::copy_internal(const QNativeIpcKey &other) void QNativeIpcKey::copy_internal(const QNativeIpcKey &other)
{ {
auto copy = new QNativeIpcKeyPrivate(*other.d_func()); d = new QNativeIpcKeyPrivate(*other.d);
d = quintptr(copy) & 1;
} }
void QNativeIpcKey::move_internal(QNativeIpcKey &&) noexcept void QNativeIpcKey::move_internal(QNativeIpcKey &&) noexcept
@ -386,21 +385,13 @@ void QNativeIpcKey::move_internal(QNativeIpcKey &&) noexcept
QNativeIpcKey &QNativeIpcKey::assign_internal(const QNativeIpcKey &other) QNativeIpcKey &QNativeIpcKey::assign_internal(const QNativeIpcKey &other)
{ {
QNativeIpcKeyPrivate *us = (d & 1) ? d_func() : nullptr; Q_ASSERT(d || other.d); // only 3 cases to handle
const QNativeIpcKeyPrivate *them = (other.d & 1) ? other.d_func() : nullptr; if (d && !other.d)
if (us && !them) { *d = {};
// don't need the extra info, reset to skinny object else if (d)
typeAndFlags = {}; *d = *other.d;
typeAndFlags.type = us->type; else
delete us; d = new QNativeIpcKeyPrivate(*other.d);
} else {
// do need the extra info, so create if necessary
if (us)
*us = *them;
else
us = new QNativeIpcKeyPrivate(*them);
d = quintptr(us) | 1;
}
return *this; return *this;
} }
@ -411,7 +402,6 @@ QNativeIpcKey &QNativeIpcKey::assign_internal(const QNativeIpcKey &other)
*/ */
void QNativeIpcKey::destroy_internal() noexcept void QNativeIpcKey::destroy_internal() noexcept
{ {
Q_D(QNativeIpcKey);
delete d; delete d;
} }
@ -458,11 +448,6 @@ void QNativeIpcKey::destroy_internal() noexcept
\sa nativeKey(), setType() \sa nativeKey(), setType()
*/ */
QNativeIpcKey::Type QNativeIpcKey::type_internal() const noexcept
{
Q_D(const QNativeIpcKey);
return d->type;
}
/*! /*!
\fn QNativeIpcKey::setType(Type type) \fn QNativeIpcKey::setType(Type type)
@ -473,8 +458,7 @@ QNativeIpcKey::Type QNativeIpcKey::type_internal() const noexcept
*/ */
void QNativeIpcKey::setType_internal(Type type) void QNativeIpcKey::setType_internal(Type type)
{ {
Q_D(QNativeIpcKey); Q_UNUSED(type);
d->type = type;
} }
/*! /*!
@ -520,7 +504,8 @@ size_t qHash(const QNativeIpcKey &ipcKey, size_t seed) noexcept
*/ */
int QNativeIpcKey::compare_internal(const QNativeIpcKey &lhs, const QNativeIpcKey &rhs) noexcept int QNativeIpcKey::compare_internal(const QNativeIpcKey &lhs, const QNativeIpcKey &rhs) noexcept
{ {
return *lhs.d_func() == *rhs.d_func() ? 0 : 1; Q_UNUSED(lhs); Q_UNUSED(rhs);
return 0;
} }
/*! /*!

View File

@ -18,7 +18,7 @@ class QNativeIpcKey
{ {
Q_GADGET_EXPORT(Q_CORE_EXPORT) Q_GADGET_EXPORT(Q_CORE_EXPORT)
public: public:
enum class Type : quintptr { enum class Type : quint16 {
// 0 is reserved for the invalid type // 0 is reserved for the invalid type
// keep 1 through 0xff free, except for SystemV // keep 1 through 0xff free, except for SystemV
SystemV = 0x51, // 'Q' SystemV = 0x51, // 'Q'
@ -37,31 +37,28 @@ public:
; ;
static Type legacyDefaultTypeForOs() noexcept; static Type legacyDefaultTypeForOs() noexcept;
constexpr QNativeIpcKey() noexcept constexpr QNativeIpcKey() noexcept = default;
: QNativeIpcKey(DefaultTypeForOs)
{}
explicit constexpr QNativeIpcKey(Type type) noexcept explicit constexpr QNativeIpcKey(Type type) noexcept
: d() : typeAndFlags{type}
{ {
typeAndFlags.type = type;
} }
Q_IMPLICIT QNativeIpcKey(const QString &k, Type type = DefaultTypeForOs) Q_IMPLICIT QNativeIpcKey(const QString &k, Type type = DefaultTypeForOs)
: d(), key(k) : key(k), typeAndFlags{type}
{ {
typeAndFlags.type = type;
} }
QNativeIpcKey(const QNativeIpcKey &other) QNativeIpcKey(const QNativeIpcKey &other)
: d(other.d), key(other.key) : d(other.d), key(other.key), typeAndFlags(other.typeAndFlags)
{ {
if (isSlowPath()) if (isSlowPath())
copy_internal(other); copy_internal(other);
} }
QNativeIpcKey(QNativeIpcKey &&other) noexcept QNativeIpcKey(QNativeIpcKey &&other) noexcept
: d(std::exchange(other.d, 0)), key(std::move(other.key)) : d(std::exchange(other.d, nullptr)), key(std::move(other.key)),
typeAndFlags(std::move(other.typeAndFlags))
{ {
if (isSlowPath()) if (isSlowPath())
move_internal(std::move(other)); move_internal(std::move(other));
@ -75,10 +72,11 @@ public:
QNativeIpcKey &operator=(const QNativeIpcKey &other) QNativeIpcKey &operator=(const QNativeIpcKey &other)
{ {
typeAndFlags = other.typeAndFlags;
key = other.key; key = other.key;
if (isSlowPath() || other.isSlowPath()) if (isSlowPath() || other.isSlowPath())
return assign_internal(other); return assign_internal(other);
d = other.d; Q_ASSERT(!d);
return *this; return *this;
} }
@ -87,6 +85,7 @@ public:
{ {
std::swap(d, other.d); std::swap(d, other.d);
key.swap(other.key); key.swap(other.key);
typeAndFlags.swap(other.typeAndFlags);
} }
bool isEmpty() const noexcept bool isEmpty() const noexcept
@ -101,8 +100,6 @@ public:
constexpr Type type() const noexcept constexpr Type type() const noexcept
{ {
if (isSlowPath())
return type_internal();
return typeAndFlags.type; return typeAndFlags.type;
} }
@ -114,7 +111,9 @@ public:
} }
QString nativeKey() const noexcept QString nativeKey() const noexcept
{ return key; } {
return key;
}
void setNativeKey(const QString &newKey) void setNativeKey(const QString &newKey)
{ {
key = newKey; key = newKey;
@ -127,36 +126,32 @@ public:
private: private:
struct TypeAndFlags { struct TypeAndFlags {
#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN Type type = DefaultTypeForOs;
// this is the LSB quint16 reserved1 = {};
quintptr isExtended : 1; quint32 reserved2 = {};
Type type : 15;
#endif
quintptr reserved : sizeof(quintptr) * 8 - 16; void swap(TypeAndFlags &other) noexcept
{
std::swap(type, other.type);
std::swap(reserved1, other.reserved1);
std::swap(reserved2, other.reserved2);
}
#if Q_BYTE_ORDER == Q_BIG_ENDIAN friend constexpr bool operator==(const TypeAndFlags &lhs, const TypeAndFlags &rhs) noexcept
Type type : 15; {
quint16 isExtended : 1; return lhs.type == rhs.type &&
// this was the LSB lhs.reserved1 == rhs.reserved1 &&
#endif lhs.reserved2 == rhs.reserved2;
}; }
// Bit 0: if set, holds a pointer (with the LSB set); if clear, holds the
// the TypeAndFlags structure.
union {
quintptr d = 0;
TypeAndFlags typeAndFlags;
static_assert(sizeof(typeAndFlags) == sizeof(d));
}; };
QNativeIpcKeyPrivate *d = nullptr;
QString key; QString key;
TypeAndFlags typeAndFlags;
friend class QNativeIpcKeyPrivate; friend class QNativeIpcKeyPrivate;
QNativeIpcKeyPrivate *d_func();
const QNativeIpcKeyPrivate *d_func() const;
constexpr bool isSlowPath() const noexcept constexpr bool isSlowPath() const noexcept
{ return Q_UNLIKELY(typeAndFlags.isExtended); } { return Q_UNLIKELY(d); }
friend Q_CORE_EXPORT size_t qHash(const QNativeIpcKey &ipcKey, size_t seed) noexcept; friend Q_CORE_EXPORT size_t qHash(const QNativeIpcKey &ipcKey, size_t seed) noexcept;
friend size_t qHash(const QNativeIpcKey &ipcKey) noexcept friend size_t qHash(const QNativeIpcKey &ipcKey) noexcept
@ -164,13 +159,13 @@ private:
friend bool operator==(const QNativeIpcKey &lhs, const QNativeIpcKey &rhs) noexcept friend bool operator==(const QNativeIpcKey &lhs, const QNativeIpcKey &rhs) noexcept
{ {
if (!(lhs.typeAndFlags == rhs.typeAndFlags))
return false;
if (lhs.key != rhs.key) if (lhs.key != rhs.key)
return false; return false;
if (lhs.d == rhs.d) if (lhs.d == rhs.d)
return true; return true;
if (lhs.isSlowPath() && rhs.isSlowPath()) return compare_internal(lhs, rhs) == 0;
return compare_internal(lhs, rhs) == 0;
return false;
} }
friend bool operator!=(const QNativeIpcKey &lhs, const QNativeIpcKey &rhs) noexcept friend bool operator!=(const QNativeIpcKey &lhs, const QNativeIpcKey &rhs) noexcept
{ {
@ -181,7 +176,6 @@ private:
Q_CORE_EXPORT void move_internal(QNativeIpcKey &&other) noexcept; Q_CORE_EXPORT void move_internal(QNativeIpcKey &&other) noexcept;
Q_CORE_EXPORT QNativeIpcKey &assign_internal(const QNativeIpcKey &other); Q_CORE_EXPORT QNativeIpcKey &assign_internal(const QNativeIpcKey &other);
Q_CORE_EXPORT void destroy_internal() noexcept; Q_CORE_EXPORT void destroy_internal() noexcept;
Q_DECL_PURE_FUNCTION Q_CORE_EXPORT Type type_internal() const noexcept;
Q_CORE_EXPORT void setType_internal(Type); Q_CORE_EXPORT void setType_internal(Type);
Q_CORE_EXPORT void setNativeKey_internal(const QString &); Q_CORE_EXPORT void setNativeKey_internal(const QString &);
Q_DECL_PURE_FUNCTION Q_CORE_EXPORT static int Q_DECL_PURE_FUNCTION Q_CORE_EXPORT static int

View File

@ -31,26 +31,8 @@ QT_BEGIN_NAMESPACE
class QNativeIpcKeyPrivate class QNativeIpcKeyPrivate
{ {
public: public:
QNativeIpcKey::Type type = {};
friend bool operator==(const QNativeIpcKeyPrivate &lhs, const QNativeIpcKeyPrivate &rhs)
{
return lhs.type == rhs.type;
}
}; };
inline QNativeIpcKeyPrivate *QNativeIpcKey::d_func()
{
Q_ASSERT(d & 1); // Q_ASSERT(isSlowPath) but without the unlikely
return reinterpret_cast<QNativeIpcKeyPrivate *>(d & ~1);
}
inline const QNativeIpcKeyPrivate *QNativeIpcKey::d_func() const
{
Q_ASSERT(d & 1); // Q_ASSERT(isSlowPath) but without the unlikely
return reinterpret_cast<QNativeIpcKeyPrivate *>(d & ~1);
}
namespace QtIpcCommon { namespace QtIpcCommon {
enum class IpcType { enum class IpcType {
SharedMemory, SharedMemory,