From a5a859e721e7a1d0c5a3ec6abe2db55d9144bb36 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Thu, 3 Jan 2019 16:02:09 +0100 Subject: [PATCH] Get rid of the connectedSignals bitflags Measurements show that it's just almost as fast to simply query the connectionlist directly and avoid both the memory overhead of the bitfield and the associated bookkeeping. For connected signals, the difference is not relevant at all. With a signal that was never connected, removing the bitfield will cause signal emission to be ~2.5% faster. And if you ever disconnect from a signal, the bitfields might not be accurate and this can cause a major slowdown. Here are some numbers to validate this. All times are measured in ms for 100M signal emissions: without change with change string based connect: 3817 3836 pointer based connect: 4552 4571 not connected: 493 479 disconnected: 2113 559 Change-Id: Ia2c85036afaa7f991b883c8ff812f69cf4580f7e Reviewed-by: Olivier Goffart (Woboq GmbH) Reviewed-by: hjk --- src/corelib/global/qhooks.cpp | 2 +- src/corelib/kernel/qobject.cpp | 65 +++++++++---------- src/corelib/kernel/qobject_p.h | 18 +---- .../other/toolsupport/tst_toolsupport.cpp | 4 +- 4 files changed, 35 insertions(+), 54 deletions(-) diff --git a/src/corelib/global/qhooks.cpp b/src/corelib/global/qhooks.cpp index bbddb1cbf1e..020dee37106 100644 --- a/src/corelib/global/qhooks.cpp +++ b/src/corelib/global/qhooks.cpp @@ -67,7 +67,7 @@ quintptr Q_CORE_EXPORT qtHookData[] = { // The required sizes and offsets are tested in tests/auto/other/toolsupport. // When this fails and the change was intentional, adjust the test and // adjust this value here. - 16 + 17 }; Q_STATIC_ASSERT(QHooks::LastHookIndex == sizeof(qtHookData) / sizeof(qtHookData[0])); diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 8a03fe2a86c..c158cf94528 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -232,7 +232,6 @@ QObjectPrivate::QObjectPrivate(int version) receiveChildEvents = true; postedEvents = 0; extraData = 0; - connectedSignals[0] = connectedSignals[1] = 0; metaObject = 0; isWindow = false; deleteLaterCalled = false; @@ -409,19 +408,12 @@ void QObjectPrivate::addConnection(int signal, Connection *c) *c->prev = c; if (c->next) c->next->prev = &c->next; - - if (signal < 0) { - connectedSignals[0] = connectedSignals[1] = ~0; - } else if (signal < (int)sizeof(connectedSignals) * 8) { - connectedSignals[signal >> 5] |= (1 << (signal & 0x1f)); - } } void QObjectPrivate::cleanConnectionLists() { if (connectionLists->dirty && !connectionLists->inUse) { // remove broken connections - bool allConnected = false; for (int signal = -1; signal < connectionLists->count(); ++signal) { QObjectPrivate::ConnectionList &connectionList = (*connectionLists)[signal]; @@ -433,13 +425,11 @@ void QObjectPrivate::cleanConnectionLists() QObjectPrivate::Connection **prev = &connectionList.first; QObjectPrivate::Connection *c = *prev; - bool connected = false; // whether the signal is still connected somewhere while (c) { if (c->receiver) { last = c; prev = &c->nextConnectionList; c = *prev; - connected = true; } else { QObjectPrivate::Connection *next = c->nextConnectionList; *prev = next; @@ -451,19 +441,40 @@ void QObjectPrivate::cleanConnectionLists() // Correct the connection list's last pointer. // As conectionList.last could equal last, this could be a noop connectionList.last = last; - - if (!allConnected && !connected && signal >= 0 - && size_t(signal) < sizeof(connectedSignals) * 8) { - // This signal is no longer connected - connectedSignals[signal >> 5] &= ~(1 << (signal & 0x1f)); - } else if (signal == -1) { - allConnected = connected; - } } connectionLists->dirty = false; } } +/*! \internal + + Returns \c true if the signal with index \a signal_index from object \a sender is connected. + + \a signal_index must be the index returned by QObjectPrivate::signalIndex; +*/ +bool QObjectPrivate::isSignalConnected(uint signalIndex, bool checkDeclarative) const +{ + if (checkDeclarative && isDeclarativeSignalConnected(signalIndex)) + return true; + + if (!connectionLists) + return false; + + if (connectionLists->allsignals.first) + return true; + + if (signalIndex < uint(connectionLists->count())) { + const QObjectPrivate::Connection *c = connectionLists->at(signalIndex).first; + while (c) { + if (c->receiver) + return true; + c = c->nextConnectionList; + } + } + return false; +} + + /*! \internal */ @@ -2521,21 +2532,7 @@ bool QObject::isSignalConnected(const QMetaMethod &signal) const signalIndex += QMetaObjectPrivate::signalOffset(signal.mobj); QMutexLocker locker(signalSlotLock(this)); - if (d->connectionLists) { - if (signalIndex < sizeof(d->connectedSignals) * 8 && !d->connectionLists->dirty) - return d->isSignalConnected(signalIndex); - - if (signalIndex < uint(d->connectionLists->count())) { - const QObjectPrivate::Connection *c = - d->connectionLists->at(signalIndex).first; - while (c) { - if (c->receiver) - return true; - c = c->nextConnectionList; - } - } - } - return d->isDeclarativeSignalConnected(signalIndex); + return d->isSignalConnected(signalIndex, true); } /*! @@ -3668,7 +3665,7 @@ void QMetaObject::activate(QObject *sender, int signalOffset, int local_signal_i Q_TRACE(QMetaObject_activate_end_declarative_signal, sender, signal_index); } - if (!sender->d_func()->isSignalConnected(signal_index, /*checkDeclarative =*/ false)) { + if (!sender->d_func()->isSignalConnected(signal_index, false)) { // The possible declarative connection is done, and nothing else is connected if (qt_signal_spy_callback_set.signal_begin_callback != nullptr) qt_signal_spy_callback_set.signal_begin_callback(sender, signal_index, argv); diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h index a762e6f5299..7cc82d4f71c 100644 --- a/src/corelib/kernel/qobject_p.h +++ b/src/corelib/kernel/qobject_p.h @@ -201,7 +201,7 @@ public: static const QObjectPrivate *get(const QObject *o) { return o->d_func(); } int signalIndex(const char *signalName, const QMetaObject **meta = nullptr) const; - inline bool isSignalConnected(uint signalIdx, bool checkDeclarative = true) const; + bool isSignalConnected(uint signalIdx, bool checkDeclarative = true) const; inline bool isDeclarativeSignalConnected(uint signalIdx) const; // To allow abitrary objects to call connectNotify()/disconnectNotify() without making @@ -232,7 +232,6 @@ public: Connection *senders; // linked list of connections connected to this object Sender *currentSender; // object currently activating the object - mutable quint32 connectedSignals[2]; union { QObject *currentChildBeingDeleted; // should only be used when QObjectData::isDeletingChildren is set @@ -246,21 +245,6 @@ public: Q_DECLARE_TYPEINFO(QObjectPrivate::ConnectionList, Q_MOVABLE_TYPE); -/*! \internal - - Returns \c true if the signal with index \a signal_index from object \a sender is connected. - Signals with indices above a certain range are always considered connected (see connectedSignals - in QObjectPrivate). - - \a signal_index must be the index returned by QObjectPrivate::signalIndex; -*/ -inline bool QObjectPrivate::isSignalConnected(uint signal_index, bool checkDeclarative) const -{ - return signal_index >= sizeof(connectedSignals) * 8 - || (connectedSignals[signal_index >> 5] & (1 << (signal_index & 0x1f)) - || (checkDeclarative && isDeclarativeSignalConnected(signal_index))); -} - inline bool QObjectPrivate::isDeclarativeSignalConnected(uint signal_index) const { return declarativeData && QAbstractDeclarativeData::isSignalConnected diff --git a/tests/auto/other/toolsupport/tst_toolsupport.cpp b/tests/auto/other/toolsupport/tst_toolsupport.cpp index f31a755f9ec..27bc6ba646b 100644 --- a/tests/auto/other/toolsupport/tst_toolsupport.cpp +++ b/tests/auto/other/toolsupport/tst_toolsupport.cpp @@ -126,9 +126,9 @@ void tst_toolsupport::offsets_data() #ifdef Q_PROCESSOR_X86 // x86 32-bit has weird alignment rules. Refer to QtPrivate::AlignOf in // qglobal.h for more details. - data << 168 << 248; + data << 160 << 240; #else - data << 172 << 248; + data << 164 << 240; #endif } #endif