Call QObject::disconnectNotify() when receiver is destroyed

Store the signal index in QObjectPrivate::Connection, thereby making
it available in "implicit" disconnect contexts (i.e., receiver
deletion).

This change does not cause the size of QObjectPrivate::Connection
to grow (still 40 bytes on 32-bit Linux, 72 bytes on 64-bit Mac).

Valgrinding the new benchmark indicates that the percentage of the
time spent in the QObject destructor increased from 7.8% to 8.4%
on ia32, for that particular stress test; the increase is the
combined cost of calling metaObject(), QMetaObjectPrivate::signal(),
and disconnectNotify() for one connection. In practice, the measured
wallclock time increased by about 3ms for a 500ms run (which
repeatedly constructs, connects, and destroys an object).

Task-number: QTBUG-4844

Change-Id: I1beb01c753f31542fc0acb62edb4c6d165fcc5b4
Reviewed-by: Olivier Goffart <ogoffart@woboq.com>
Reviewed-by: Bradley T. Hughes <bradley.hughes@nokia.com>

diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index bcbc45c..c0bb812 100644 (file)
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -831,9 +831,11 @@ QObject::~QObject()
             if (senderLists)
                 senderLists->dirty = true;
 
+            int signal_index = node->signal_index;
             node = node->next;
             if (needToUnlock)
                 m->unlock();
+            sender->disconnectNotify(QMetaObjectPrivate::signal(sender->metaObject(), signal_index));
         }
     }
 
@@ -3021,6 +3023,7 @@ QObjectPrivate::Connection *QMetaObjectPrivate::connect(const QObject *sender,
 
     QScopedPointer<QObjectPrivate::Connection> c(new QObjectPrivate::Connection);
     c->sender = s;
+    c->signal_index = signal_index;
     c->receiver = r;
     c->method_relative = method_index;
     c->method_offset = method_offset;
@@ -4188,6 +4191,7 @@ QMetaObject::Connection QObject::connectImpl(const QObject *sender, void **signa
 
     QScopedPointer<QObjectPrivate::Connection> c(new QObjectPrivate::Connection);
     c->sender = s;
+    c->signal_index = signal_index;
     c->receiver = r;
     c->slotObj = slotObj;
     c->connectionType = type;

diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h
index 0d491a2..446e920 100644 (file)
--- a/src/corelib/kernel/qobject_p.h
+++ b/src/corelib/kernel/qobject_p.h
@@ -131,6 +131,7 @@ public:
         QAtomicInt ref_;
         ushort method_offset;
         ushort method_relative;
+        uint signal_index : 27; // In signal range (see QObjectPrivate::signalIndex())
         ushort connectionType : 3; // 0 == auto, 1 == direct, 2 == queued, 4 == blocking
         ushort isSlotObject : 1;
         ushort ownArgumentTypes : 1;

diff --git a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
index 0cdce50..479cdd3 100644 (file)
--- a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
+++ b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
@@ -978,8 +978,26 @@ void tst_QObject::disconnectNotify_receiverDestroyed()
     QVERIFY(QObject::connect((SenderObject*)s, SIGNAL(signal1()), (ReceiverObject*)r, SLOT(slot1())));
 
     delete r;
-    // disconnectNotify() is not called, but it probably should be.
-    QVERIFY(s->disconnectedSignals.isEmpty());
+    QCOMPARE(s->disconnectedSignals.count(), 1);
+    QCOMPARE(s->disconnectedSignals.at(0), QMetaMethod::fromSignal(&SenderObject::signal1));
+
+    s->disconnectedSignals.clear();
+    r = new NotifyObject;
+
+    QVERIFY(QObject::connect((SenderObject*)s, SIGNAL(signal3()), (ReceiverObject*)r, SLOT(slot3())));
+
+    delete r;
+    QCOMPARE(s->disconnectedSignals.count(), 1);
+    QCOMPARE(s->disconnectedSignals.at(0), QMetaMethod::fromSignal(&SenderObject::signal3));
+
+    s->disconnectedSignals.clear();
+    r = new NotifyObject;
+
+    QVERIFY(QObject::connect((SenderObject*)s, SIGNAL(destroyed()), (ReceiverObject*)r, SLOT(slot3())));
+
+    delete r;
+    QCOMPARE(s->disconnectedSignals.count(), 1);
+    QCOMPARE(s->disconnectedSignals.at(0), QMetaMethod::fromSignal(&QObject::destroyed));
 
     delete s;
 }

diff --git a/tests/benchmarks/corelib/kernel/qobject/main.cpp b/tests/benchmarks/corelib/kernel/qobject/main.cpp
index a5a26c1..90912d4 100644 (file)
--- a/tests/benchmarks/corelib/kernel/qobject/main.cpp
+++ b/tests/benchmarks/corelib/kernel/qobject/main.cpp
@@ -61,6 +61,7 @@ private slots:
     void dynamic_property_benchmark();
     void connect_disconnect_benchmark_data();
     void connect_disconnect_benchmark();
+    void receiver_destroyed_benchmark();
 };
 
 struct Functor {
@@ -236,6 +237,15 @@ void QObjectBenchmark::connect_disconnect_benchmark()
     }
 }
 
+void QObjectBenchmark::receiver_destroyed_benchmark()
+{
+    Object sender;
+    QBENCHMARK {
+        Object receiver;
+        QObject::connect(&sender, &Object::signal0, &receiver, &Object::slot0);
+    }
+}
+
 QTEST_MAIN(QObjectBenchmark)
 
 #include "main.moc"