From 386c640d9c0cc0984f33225b72e6d46e3718dcf7 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Mon, 11 May 2009 11:22:59 +0200 Subject: Faster slot disconnection while destroying of QObject Destroying a QObject which is connected to many signals was slow. We had to loop over all the ConnectionsList of all the connected object to remove the connections. (in QObjectPrivate::removeReceiver) The idea is to now use lists of pointer to Connection both in the connection list, and in the list of senders. So clearing the connection should be faster. Task-number: 251097 Reviewed-by: Brad --- src/corelib/kernel/qobject.cpp | 226 +++++++++++++++-------------------------- src/corelib/kernel/qobject_p.h | 11 +- 2 files changed, 87 insertions(+), 150 deletions(-) diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 05015c0..625abd7 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -70,7 +70,7 @@ static int DIRECT_CONNECTION_ONLY = 0; static int *queuedConnectionTypes(const QList &typeNames) { - int *types = static_cast(qMalloc((typeNames.count() + 1) * sizeof(int))); + int *types = new int [typeNames.count() + 1]; for (int i = 0; i < typeNames.count(); ++i) { const QByteArray typeName = typeNames.at(i); if (typeName.endsWith('*')) @@ -82,7 +82,7 @@ static int *queuedConnectionTypes(const QList &typeNames) qWarning("QObject::connect: Cannot queue arguments of type '%s'\n" "(Make sure '%s' is registered using qRegisterMetaType().)", typeName.constData(), typeName.constData()); - qFree(types); + delete [] types; return 0; } } @@ -216,6 +216,7 @@ public: } }; +// Used by QAccessibleWidget bool QObjectPrivate::isSender(const QObject *receiver, const char *signal) const { Q_Q(const QObject); @@ -227,8 +228,8 @@ bool QObjectPrivate::isSender(const QObject *receiver, const char *signal) const if (signal_index < connectionLists->count()) { const ConnectionList &connectionList = connectionLists->at(signal_index); for (int i = 0; i < connectionList.count(); ++i) { - const QObjectPrivate::Connection &c = connectionList.at(i); - if (c.receiver && c.receiver == receiver) + const QObjectPrivate::Connection *c = connectionList.at(i); + if (c->receiver == receiver) return true; } } @@ -236,6 +237,7 @@ bool QObjectPrivate::isSender(const QObject *receiver, const char *signal) const return false; } +// Used by QAccessibleWidget QObjectList QObjectPrivate::receiverList(const char *signal) const { Q_Q(const QObject); @@ -248,21 +250,22 @@ QObjectList QObjectPrivate::receiverList(const char *signal) const if (signal_index < connectionLists->count()) { const ConnectionList &connectionList = connectionLists->at(signal_index); for (int i = 0; i < connectionList.count(); ++i) { - const QObjectPrivate::Connection &c = connectionList.at(i); - if (c.receiver) - returnValue << c.receiver; + const QObjectPrivate::Connection *c = connectionList.at(i); + if (c->receiver) + returnValue << c->receiver; } } } return returnValue; } +// Used by QAccessibleWidget QObjectList QObjectPrivate::senderList() const { QObjectList returnValue; QMutexLocker locker(&threadData->mutex); for (int i = 0; i < senders.count(); ++i) - returnValue << senders.at(i).sender; + returnValue << senders.at(i)->sender; return returnValue; } @@ -274,33 +277,11 @@ void QObjectPrivate::addConnection(int signal, Connection *c) connectionLists->resize(signal + 1); ConnectionList &connectionList = (*connectionLists)[signal]; - connectionList.append(*c); + connectionList.append(c); cleanConnectionLists(); } -void QObjectPrivate::removeReceiver(int signal, QObject *receiver) -{ - if (!connectionLists) - return; - - if (signal >= connectionLists->count()) - return; - - ConnectionList &connectionList = (*connectionLists)[signal]; - for (int i = 0; i < connectionList.count(); ++i) { - Connection &c = connectionList[i]; - if (c.receiver == receiver) { - c.receiver = 0; - if (c.argumentTypes && c.argumentTypes != &DIRECT_CONNECTION_ONLY) { - qFree(c.argumentTypes); - c.argumentTypes = 0; - } - connectionLists->dirty = true; - } - } -} - void QObjectPrivate::cleanConnectionLists() { if (connectionLists->dirty && !connectionLists->inUse) { @@ -308,55 +289,17 @@ void QObjectPrivate::cleanConnectionLists() for (int signal = -1; signal < connectionLists->count(); ++signal) { QObjectPrivate::ConnectionList &connectionList = (*connectionLists)[signal]; for (int i = 0; i < connectionList.count(); ++i) { - const QObjectPrivate::Connection &c = connectionList.at(i); - if (!c.receiver) + QObjectPrivate::Connection *c = connectionList.at(i); + if (!c->receiver) { + delete c; connectionList.removeAt(i--); + } } } connectionLists->dirty = false; } } -void QObjectPrivate::refSender(QObject *sender, int signal) -{ - for (int i = 0; i < senders.count(); ++i) { - Sender &s = senders[i]; - if (s.sender == sender && s.signal == signal) { - ++s.ref; - return; - } - } - - Sender s = { sender, signal, 1 }; - senders.append(s); -} - -void QObjectPrivate::derefSender(QObject *sender, int signal) -{ - for (int i = 0; i < senders.count(); ++i) { - Sender &s = senders[i]; - if (s.sender == sender && s.signal == signal) { - if (--s.ref == 0) { - senders.removeAt(i); - break; - } - } - } - // Q_ASSERT_X(false, "QObjectPrivate::derefSender", "sender not found"); -} - -void QObjectPrivate::removeSender(QObject *sender, int signal) -{ - for (int i = 0; i < senders.count(); ++i) { - Sender &s = senders[i]; - if (s.sender == sender && s.signal == signal) { - senders.removeAt(i); - return; - } - } - // Q_ASSERT_X(false, "QObjectPrivate::removeSender", "sender not found"); -} - QObjectPrivate::Sender *QObjectPrivate::setCurrentSender(QObject *receiver, Sender *sender) { @@ -782,23 +725,21 @@ QObject::~QObject() for (int signal = -1; signal < d->connectionLists->count(); ++signal) { QObjectPrivate::ConnectionList &connectionList = (*d->connectionLists)[signal]; for (int i = 0; i < connectionList.count(); ++i) { - QObjectPrivate::Connection *c = &connectionList[i]; - if (!c->receiver) + QObjectPrivate::Connection *c = connectionList[i]; + if (!c->receiver) { + delete c; continue; + } QMutex *m = &c->receiver->d_func()->threadData->mutex; bool needToUnlock = QOrderedMutexLocker::relock(locker.mutex(), m); - c = &connectionList[i]; + c = connectionList[i]; if (c->receiver) - c->receiver->d_func()->removeSender(this, signal); + c->receiver->d_func()->senders.removeOne(c); if (needToUnlock) m->unlock(); - if (c->argumentTypes && c->argumentTypes != &DIRECT_CONNECTION_ONLY) { - qFree(c->argumentTypes); - c->argumentTypes = 0; - } - c->receiver = 0; + delete c; } } @@ -812,15 +753,20 @@ QObject::~QObject() // disconnect all senders for (int i = 0; i < d->senders.count(); ++i) { - QObjectPrivate::Sender *s = &d->senders[i]; + QObjectPrivate::Connection *s = d->senders[i]; if (!s->sender) continue; QMutex *m = &s->sender->d_func()->threadData->mutex; bool needToUnlock = QOrderedMutexLocker::relock(locker.mutex(), m); - s = &d->senders[i]; - if (s->sender) - s->sender->d_func()->removeReceiver(s->signal, this); + s = d->senders[i]; + s->receiver = 0; + if (s->sender) { + QObjectConnectionListVector *senderLists = s->sender->d_func()->connectionLists; + if (senderLists) + senderLists->dirty = true; + } + if (needToUnlock) m->unlock(); } @@ -866,6 +812,12 @@ QObject::~QObject() d_ptr = 0; } +QObjectPrivate::Connection::~Connection() +{ + if (argumentTypes != &DIRECT_CONNECTION_ONLY) + delete [] argumentTypes; +} + /*! \fn QMetaObject *QObject::metaObject() const @@ -2304,7 +2256,7 @@ QObject *QObject::sender() const // Return 0 if d->currentSender isn't in d->senders bool found = false; for (int i = 0; !found && i < d->senders.count(); ++i) - found = (d->senders.at(i).sender == d->currentSender->sender); + found = (d->senders.at(i)->sender == d->currentSender->sender); if (!found) return 0; return d->currentSender->sender; @@ -2359,8 +2311,8 @@ int QObject::receivers(const char *signal) const const QObjectPrivate::ConnectionList &connectionList = d->connectionLists->at(signal_index); for (int i = 0; i < connectionList.count(); ++i) { - const QObjectPrivate::Connection &c = connectionList.at(i); - receivers += c.receiver ? 1 : 0; + const QObjectPrivate::Connection *c = connectionList.at(i); + receivers += c->receiver ? 1 : 0; } } } @@ -2793,20 +2745,18 @@ bool QMetaObject::connect(const QObject *sender, int signal_index, QObject *s = const_cast(sender); QObject *r = const_cast(receiver); + QObjectPrivate::Connection *c = new QObjectPrivate::Connection; + c->sender = s; + c->receiver = r; + c->method = method_index; + c->connectionType = type; + c->argumentTypes = types; + QOrderedMutexLocker locker(&s->d_func()->threadData->mutex, &r->d_func()->threadData->mutex); -#if defined(Q_CC_HPACC) && defined(QT_ARCH_PARISC) - QObjectPrivate::Connection c; - c.receiver = r; - c.method = method_index; - c.connectionType = type; - c.argumentTypes = types; -#else - QObjectPrivate::Connection c = { r, method_index, type, Q_BASIC_ATOMIC_INITIALIZER(types) }; -#endif - s->d_func()->addConnection(signal_index, &c); - r->d_func()->refSender(s, signal_index); + s->d_func()->addConnection(signal_index, c); + r->d_func()->senders.append(c); if (signal_index < 0) sender->d_func()->connectedSignals = ~0u; @@ -2845,27 +2795,23 @@ bool QMetaObject::disconnect(const QObject *sender, int signal_index, for (signal_index = -1; signal_index < connectionLists->count(); ++signal_index) { QObjectPrivate::ConnectionList &connectionList = (*connectionLists)[signal_index]; for (int i = 0; i < connectionList.count(); ++i) { - QObjectPrivate::Connection *c = &connectionList[i]; + QObjectPrivate::Connection *c = connectionList[i]; if (c->receiver && (r == 0 || (c->receiver == r && (method_index < 0 || c->method == method_index)))) { QMutex *m = &c->receiver->d_func()->threadData->mutex; + bool needToUnlock = false; if (!receiverMutex && senderMutex != m) { // need to relock this receiver and sender in the correct order - bool needToUnlock = QOrderedMutexLocker::relock(senderMutex, m); - c = &connectionList[i]; - if (c->receiver) - c->receiver->d_func()->derefSender(s, signal_index); - if (needToUnlock) - m->unlock(); - } else { - // no need to unlock - c->receiver->d_func()->derefSender(s, signal_index); - } - if (c->argumentTypes && c->argumentTypes != &DIRECT_CONNECTION_ONLY) { - qFree(c->argumentTypes); - c->argumentTypes = 0; + needToUnlock = QOrderedMutexLocker::relock(senderMutex, m); + c = connectionList[i]; } + if (c->receiver) + c->receiver->d_func()->senders.removeOne(c); + + if (needToUnlock) + m->unlock(); + c->receiver = 0; success = true; @@ -2876,27 +2822,22 @@ bool QMetaObject::disconnect(const QObject *sender, int signal_index, } else if (signal_index < connectionLists->count()) { QObjectPrivate::ConnectionList &connectionList = (*connectionLists)[signal_index]; for (int i = 0; i < connectionList.count(); ++i) { - QObjectPrivate::Connection *c = &connectionList[i]; + QObjectPrivate::Connection *c = connectionList[i]; if (c->receiver && (r == 0 || (c->receiver == r && (method_index < 0 || c->method == method_index)))) { QMutex *m = &c->receiver->d_func()->threadData->mutex; + bool needToUnlock = false; if (!receiverMutex && senderMutex != m) { // need to relock this receiver and sender in the correct order - bool needToUnlock = QOrderedMutexLocker::relock(senderMutex, m); - c = &connectionList[i]; - if (c->receiver) - c->receiver->d_func()->derefSender(s, signal_index); - if (needToUnlock) - m->unlock(); - } else { - // no need to unlock - c->receiver->d_func()->derefSender(s, signal_index); - } - if (c->argumentTypes && c->argumentTypes != &DIRECT_CONNECTION_ONLY) { - qFree(c->argumentTypes); - c->argumentTypes = 0; + needToUnlock = QOrderedMutexLocker::relock(senderMutex, m); + c = connectionList[i]; } + if (c->receiver) + c->receiver->d_func()->senders.removeOne(c); + + if (needToUnlock) + m->unlock(); c->receiver = 0; success = true; @@ -2979,32 +2920,31 @@ void QMetaObject::connectSlotsByName(QObject *o) } } -static void queued_activate(QObject *sender, int signal, const QObjectPrivate::Connection &c, +static void queued_activate(QObject *sender, int signal, QObjectPrivate::Connection *c, void **argv, QSemaphore *semaphore = 0) { - if (!c.argumentTypes || c.argumentTypes != &DIRECT_CONNECTION_ONLY) { + if (!c->argumentTypes && c->argumentTypes != &DIRECT_CONNECTION_ONLY) { QMetaMethod m = sender->metaObject()->method(signal); - QObjectPrivate::Connection &x = const_cast(c); int *tmp = queuedConnectionTypes(m.parameterTypes()); if (!tmp) // cannot queue arguments tmp = &DIRECT_CONNECTION_ONLY; - if (!x.argumentTypes.testAndSetOrdered(0, tmp)) { + if (!c->argumentTypes.testAndSetOrdered(0, tmp)) { if (tmp != &DIRECT_CONNECTION_ONLY) - qFree(tmp); + delete [] tmp; } } - if (c.argumentTypes == &DIRECT_CONNECTION_ONLY) // cannot activate + if (c->argumentTypes == &DIRECT_CONNECTION_ONLY) // cannot activate return; int nargs = 1; // include return type - while (c.argumentTypes[nargs-1]) + while (c->argumentTypes[nargs-1]) ++nargs; int *types = (int *) qMalloc(nargs*sizeof(int)); void **args = (void **) qMalloc(nargs*sizeof(void *)); types[0] = 0; // return type args[0] = 0; // return value for (int n = 1; n < nargs; ++n) - args[n] = QMetaType::construct((types[n] = c.argumentTypes[n-1]), argv[n]); - QCoreApplication::postEvent(c.receiver, new QMetaCallEvent(c.method, + args[n] = QMetaType::construct((types[n] = c->argumentTypes[n-1]), argv[n]); + QCoreApplication::postEvent(c->receiver, new QMetaCallEvent(c->method, sender, signal, nargs, @@ -3013,13 +2953,13 @@ static void queued_activate(QObject *sender, int signal, const QObjectPrivate::C semaphore)); } -static void blocking_activate(QObject *sender, int signal, const QObjectPrivate::Connection &c, void **argv) +static void blocking_activate(QObject *sender, int signal, QObjectPrivate::Connection *c, void **argv) { - if (QThread::currentThread() == c.receiver->thread()) { + if (QThread::currentThread() == c->receiver->thread()) { qWarning("Qt: Dead lock detected while activating a BlockingQueuedConnection: " "Sender is %s(%p), receiver is %s(%p)", sender->metaObject()->className(), sender, - c.receiver->metaObject()->className(), c.receiver); + c->receiver->metaObject()->className(), c->receiver); } #ifdef QT_NO_THREAD @@ -3069,7 +3009,7 @@ void QMetaObject::activate(QObject *sender, int from_signal_index, int to_signal } int count = connectionLists->at(signal).count(); for (int i = 0; i < count; ++i) { - const QObjectPrivate::Connection *c = &connectionLists->at(signal)[i]; + QObjectPrivate::Connection *c = connectionLists->at(signal)[i]; if (!c->receiver) continue; @@ -3081,10 +3021,10 @@ void QMetaObject::activate(QObject *sender, int from_signal_index, int to_signal && (currentThreadData != sender->d_func()->threadData || receiver->d_func()->threadData != sender->d_func()->threadData)) || (c->connectionType == Qt::QueuedConnection)) { - queued_activate(sender, signal, *c, argv); + queued_activate(sender, signal, c, argv); continue; } else if (c->connectionType == Qt::BlockingQueuedConnection) { - blocking_activate(sender, signal, *c, argv); + blocking_activate(sender, signal, c, argv); continue; } diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h index b324334..f70a77d 100644 --- a/src/corelib/kernel/qobject_p.h +++ b/src/corelib/kernel/qobject_p.h @@ -114,7 +114,6 @@ public: int signal; int ref; }; - // object currently activating the object Sender *currentSender; @@ -148,22 +147,20 @@ public: // Note: you must hold the signalSlotLock() before accessing the lists below or calling the functions struct Connection { + QObject *sender; QObject *receiver; int method; uint connectionType : 3; // 0 == auto, 1 == direct, 2 == queued, 4 == blocking QBasicAtomicPointer argumentTypes; + ~Connection(); }; - typedef QList ConnectionList; + typedef QList ConnectionList; QObjectConnectionListVector *connectionLists; void addConnection(int signal, Connection *c); - void removeReceiver(int signal, QObject *receiver); void cleanConnectionLists(); - QList senders; - void refSender(QObject *sender, int signal); - void derefSender(QObject *sender, int signal); - void removeSender(QObject *sender, int signal); + ConnectionList senders; static Sender *setCurrentSender(QObject *receiver, Sender *sender); -- cgit v0.12