Optimize QTimer::singleShot(0, ...) when taking PMF or Functor callable

QTimer::singleShot is optimized for zero timeouts when using the API
taking a string method name. This optimization was not used for the API
taking a PMF or functor. This patch adds it, making the various API
calls behave similarly from a performance point of view.

The approach taken here requires a QObject context object. If none is
available, e.g. a nullptr was passed explicitly, or the
QTimer::singleShot(O, Functor) API was used, the optimization could
not easily be applied. This is not only bad from a performance POV,
but also poses as a potential source for heisenbugs: Using the
different API versions of QTimer::singleShot would use different code
paths internally, which then would not ensure the expected slot call
order. This problem actually existed already when mixing the
string-based slot syntax with PMF/functors in the QTimer::singleShot
API.

This patch overcomes this hurdle and fixes all of the above: When we
encounter a 0ms single shot timer, and no QObject context object is
available, we fall back to the main thread, or create a temporary
QObject for any other thread. The updated and extended benchmark
shows that this is still a significant performance improvement
over using a timer:

********* Start testing of qtimer_vs_qmetaobject *********
Config: Using QtTest library 5.14.0, Qt 5.14.0 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 8.2.1 20181127)
PASS   : qtimer_vs_qmetaobject::initTestCase()
PASS   : qtimer_vs_qmetaobject::bench(singleShot_slot)
RESULT : qtimer_vs_qmetaobject::bench():"singleShot_slot":
     7.48 msecs per iteration (total: 748, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(singleShot_pmf)
RESULT : qtimer_vs_qmetaobject::bench():"singleShot_pmf":
     7.20 msecs per iteration (total: 720, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(singleShot_functor)
RESULT : qtimer_vs_qmetaobject::bench():"singleShot_functor":
     6.79 msecs per iteration (total: 679, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(singleShot_functor_noctx)
RESULT : qtimer_vs_qmetaobject::bench():"singleShot_functor_noctx":
     6.92 msecs per iteration (total: 693, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(invokeMethod_string)
RESULT : qtimer_vs_qmetaobject::bench():"invokeMethod_string":
     7.34 msecs per iteration (total: 735, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(invokeMethod_pmf)
RESULT : qtimer_vs_qmetaobject::bench():"invokeMethod_pmf":
     6.90 msecs per iteration (total: 690, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(invokeMethod_functor)
RESULT : qtimer_vs_qmetaobject::bench():"invokeMethod_functor":
     6.62 msecs per iteration (total: 662, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(singleShot_slot)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"singleShot_slot":
     7.45 msecs per iteration (total: 745, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(singleShot_pmf)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"singleShot_pmf":
     7.46 msecs per iteration (total: 747, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(singleShot_functor)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"singleShot_functor":
     6.70 msecs per iteration (total: 671, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(singleShot_functor_noctx)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"singleShot_functor_noctx":
     13.75 msecs per iteration (total: 1,376, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(invokeMethod_string)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"invokeMethod_string":
     7.05 msecs per iteration (total: 706, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(invokeMethod_pmf)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"invokeMethod_pmf":
     6.70 msecs per iteration (total: 670, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(invokeMethod_functor)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"invokeMethod_functor":
     6.58 msecs per iteration (total: 658, iterations: 100)
PASS   : qtimer_vs_qmetaobject::cleanupTestCase()
Totals: 16 passed, 0 failed, 0 skipped, 0 blacklisted, 20977ms
********* Finished testing of qtimer_vs_qmetaobject *********

Without the change to qtimer.cpp, the results are:

********* Start testing of qtimer_vs_qmetaobject *********
Config: Using QtTest library 5.14.0, Qt 5.14.0 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 8.2.1 20181127)
PASS   : qtimer_vs_qmetaobject::initTestCase()
PASS   : qtimer_vs_qmetaobject::bench(singleShot_slot)
RESULT : qtimer_vs_qmetaobject::bench():"singleShot_slot":
     7.45 msecs per iteration (total: 745, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(singleShot_pmf)
RESULT : qtimer_vs_qmetaobject::bench():"singleShot_pmf":
     112.84 msecs per iteration (total: 11,285, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(singleShot_functor)
RESULT : qtimer_vs_qmetaobject::bench():"singleShot_functor":
     115.62 msecs per iteration (total: 11,563, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(singleShot_functor_noctx)
RESULT : qtimer_vs_qmetaobject::bench():"singleShot_functor_noctx":
     110.81 msecs per iteration (total: 11,082, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(invokeMethod_string)
RESULT : qtimer_vs_qmetaobject::bench():"invokeMethod_string":
     7.04 msecs per iteration (total: 704, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(invokeMethod_pmf)
RESULT : qtimer_vs_qmetaobject::bench():"invokeMethod_pmf":
     6.62 msecs per iteration (total: 662, iterations: 100)
PASS   : qtimer_vs_qmetaobject::bench(invokeMethod_functor)
RESULT : qtimer_vs_qmetaobject::bench():"invokeMethod_functor":
     6.62 msecs per iteration (total: 662, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(singleShot_slot)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"singleShot_slot":
     7.45 msecs per iteration (total: 746, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(singleShot_pmf)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"singleShot_pmf":
     118.42 msecs per iteration (total: 11,842, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(singleShot_functor)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"singleShot_functor":
     119.35 msecs per iteration (total: 11,936, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(singleShot_functor_noctx)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"singleShot_functor_noctx":
     130.96 msecs per iteration (total: 13,096, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(invokeMethod_string)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"invokeMethod_string":
     8.08 msecs per iteration (total: 808, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(invokeMethod_pmf)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"invokeMethod_pmf":
     6.79 msecs per iteration (total: 680, iterations: 100)
PASS   : qtimer_vs_qmetaobject::benchBackgroundThread(invokeMethod_functor)
RESULT : qtimer_vs_qmetaobject::benchBackgroundThread():"invokeMethod_functor":
     7.49 msecs per iteration (total: 749, iterations: 100)
PASS   : qtimer_vs_qmetaobject::cleanupTestCase()
Totals: 16 passed, 0 failed, 0 skipped, 0 blacklisted, 153995ms
********* Finished testing of qtimer_vs_qmetaobject *********

Additionally, this patch adds a unit test to verify that the slot call
order for 0ms single shot timers is followed while mixing the various
API versions. It fails without this patch but passes now.

Finally, another test is added to verify that using QTimer::singleShot
before a QCoreApplication was constructed is still working properly.

Change-Id: I0d6211554b6198cb3e527be9ec3adc572b1b54ee
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

5 files changed, 182 insertions, 19 deletions

diff --git a/src/corelib/kernel/qobjectdefs.h b/src/corelib/kernel/qobjectdefs.h
index 28674193c3..418b8cf1d2 100644
--- a/src/corelib/kernel/qobjectdefs.h
+++ b/src/corelib/kernel/qobjectdefs.h
@@ -576,6 +576,7 @@ struct Q_CORE_EXPORT QMetaObject
 
 private:
     static bool invokeMethodImpl(QObject *object, QtPrivate::QSlotObjectBase *slot, Qt::ConnectionType type, void *ret);
+    friend class QTimer;
 };
 
 class Q_CORE_EXPORT QMetaObject::Connection {
diff --git a/src/corelib/kernel/qtimer.cpp b/src/corelib/kernel/qtimer.cpp
index 13f027074a..188d529f04 100644
--- a/src/corelib/kernel/qtimer.cpp
+++ b/src/corelib/kernel/qtimer.cpp
@@ -42,6 +42,8 @@
 #include "qabstracteventdispatcher.h"
 #include "qcoreapplication.h"
 #include "qobject_p.h"
+#include "qthread.h"
+#include "qcoreapplication_p.h"
 
 QT_BEGIN_NAMESPACE
 
@@ -343,6 +345,33 @@ void QTimer::singleShotImpl(int msec, Qt::TimerType timerType,
                             const QObject *receiver,
                             QtPrivate::QSlotObjectBase *slotObj)
 {
+    if (msec == 0) {
+        bool deleteReceiver = false;
+        // Optimize: set a receiver context when none is given, such that we can use
+        // QMetaObject::invokeMethod which is more efficient than going through a timer.
+        // We need a QObject living in the current thread. But the QThread itself lives
+        // in a different thread - with the exception of the main QThread which lives in
+        // itself. And QThread::currentThread() is among the few QObjects we know that will
+        // most certainly be there. Note that one can actually call singleShot before the
+        // QApplication is created!
+        if (!receiver && QThread::currentThread() == QCoreApplicationPrivate::mainThread()) {
+            // reuse main thread as context object
+            receiver = QThread::currentThread();
+        } else if (!receiver) {
+            // Create a receiver context object on-demand. According to the benchmarks,
+            // this is still more efficient than going through a timer.
+            receiver = new QObject;
+            deleteReceiver = true;
+        }
+
+        QMetaObject::invokeMethodImpl(const_cast<QObject *>(receiver), slotObj,
+                                      Qt::QueuedConnection, nullptr);
+
+        if (deleteReceiver)
+            const_cast<QObject *>(receiver)->deleteLater();
+        return;
+    }
+
     new QSingleShotTimer(msec, timerType, receiver, slotObj);
 }
 
diff --git a/src/testlib/qtest.h b/src/testlib/qtest.h
index ebd94939ce..89abc616d9 100644
--- a/src/testlib/qtest.h
+++ b/src/testlib/qtest.h
@@ -424,48 +424,45 @@ int main(int argc, char *argv[]) \
 #  define QTEST_DISABLE_KEYPAD_NAVIGATION
 #endif
 
-#define QTEST_MAIN(TestObject) \
-int main(int argc, char *argv[]) \
-{ \
+#define QTEST_MAIN_IMPL(TestObject) \
     TESTLIB_SELFCOVERAGE_START(#TestObject) \
     QApplication app(argc, argv); \
     app.setAttribute(Qt::AA_Use96Dpi, true); \
     QTEST_DISABLE_KEYPAD_NAVIGATION \
     TestObject tc; \
     QTEST_SET_MAIN_SOURCE_PATH \
-    return QTest::qExec(&tc, argc, argv); \
-}
+    return QTest::qExec(&tc, argc, argv);
 
 #elif defined(QT_GUI_LIB)
 
 #include <QtTest/qtest_gui.h>
 
-#define QTEST_MAIN(TestObject) \
-int main(int argc, char *argv[]) \
-{ \
+#define QTEST_MAIN_IMPL(TestObject) \
     TESTLIB_SELFCOVERAGE_START(#TestObject) \
     QGuiApplication app(argc, argv); \
     app.setAttribute(Qt::AA_Use96Dpi, true); \
     TestObject tc; \
     QTEST_SET_MAIN_SOURCE_PATH \
-    return QTest::qExec(&tc, argc, argv); \
-}
+    return QTest::qExec(&tc, argc, argv);
 
 #else
 
-#define QTEST_MAIN(TestObject) \
-int main(int argc, char *argv[]) \
-{ \
+#define QTEST_MAIN_IMPL(TestObject) \
     TESTLIB_SELFCOVERAGE_START(#TestObject) \
     QCoreApplication app(argc, argv); \
     app.setAttribute(Qt::AA_Use96Dpi, true); \
     TestObject tc; \
     QTEST_SET_MAIN_SOURCE_PATH \
-    return QTest::qExec(&tc, argc, argv); \
-}
+    return QTest::qExec(&tc, argc, argv);
 
 #endif // QT_GUI_LIB
 
+#define QTEST_MAIN(TestObject) \
+int main(int argc, char *argv[]) \
+{ \
+    QTEST_MAIN_IMPL(TestObject) \
+}
+
 #define QTEST_GUILESS_MAIN(TestObject) \
 int main(int argc, char *argv[]) \
 { \
diff --git a/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp b/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp
index 3b10547dc4..d06059bbe5 100644
--- a/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp
+++ b/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp
@@ -73,7 +73,12 @@ private slots:
     void recurseOnTimeoutAndStopTimer();
     void singleShotToFunctors();
     void singleShot_chrono();
+    void singleShot_static();
     void crossThreadSingleShotToFunctor();
+    void timerOrder();
+    void timerOrder_data();
+    void timerOrderBackgroundThread();
+    void timerOrderBackgroundThread_data() { timerOrder_data(); }
 
     void dontBlockEvents();
     void postedEventsShouldNotStarveTimers();
@@ -1033,5 +1038,121 @@ void tst_QTimer::callOnTimeout()
     QVERIFY(!connection);
 }
 
-QTEST_MAIN(tst_QTimer)
+class OrderHelper : public QObject
+{
+    Q_OBJECT
+public:
+    enum CallType
+    {
+        String,
+        PMF,
+        Functor,
+        FunctorNoCtx
+    };
+    Q_ENUM(CallType)
+    QVector<CallType> calls;
+
+    void triggerCall(CallType callType)
+    {
+        switch (callType)
+        {
+        case String:
+            QTimer::singleShot(0, this, SLOT(stringSlot()));
+            break;
+        case PMF:
+            QTimer::singleShot(0, this, &OrderHelper::pmfSlot);
+            break;
+        case Functor:
+            QTimer::singleShot(0, this, [this]() { functorSlot(); });
+            break;
+        case FunctorNoCtx:
+            QTimer::singleShot(0, [this]() { functorNoCtxSlot(); });
+            break;
+        }
+    }
+
+public slots:
+    void stringSlot() { calls << String; }
+    void pmfSlot() { calls << PMF; }
+    void functorSlot() { calls << Functor; }
+    void functorNoCtxSlot() { calls << FunctorNoCtx; }
+};
+
+Q_DECLARE_METATYPE(OrderHelper::CallType)
+
+void tst_QTimer::timerOrder()
+{
+    QFETCH(QVector<OrderHelper::CallType>, calls);
+
+    OrderHelper helper;
+
+    for (const auto call : calls)
+        helper.triggerCall(call);
+
+    QTRY_COMPARE(helper.calls, calls);
+}
+
+void tst_QTimer::timerOrder_data()
+{
+    QTest::addColumn<QVector<OrderHelper::CallType>>("calls");
+
+    QVector<OrderHelper::CallType> calls = {
+        OrderHelper::String, OrderHelper::PMF,
+        OrderHelper::Functor, OrderHelper::FunctorNoCtx
+    };
+    std::sort(calls.begin(), calls.end());
+
+    int permutation = 0;
+    do {
+        QTest::addRow("permutation=%d", permutation) << calls;
+        ++permutation;
+    } while (std::next_permutation(calls.begin(), calls.end()));
+}
+
+void tst_QTimer::timerOrderBackgroundThread()
+{
+#if !QT_CONFIG(cxx11_future)
+    QSKIP("This test requires QThread::create");
+#else
+    auto *thread = QThread::create([this]() { timerOrder(); });
+    thread->start();
+    QVERIFY(thread->wait());
+    delete thread;
+#endif
+}
+
+struct StaticSingleShotUser
+{
+    StaticSingleShotUser()
+    {
+        for (auto call : calls())
+            helper.triggerCall(call);
+    }
+    OrderHelper helper;
+
+    static QVector<OrderHelper::CallType> calls()
+    {
+        return {OrderHelper::String, OrderHelper::PMF,
+                OrderHelper::Functor, OrderHelper::FunctorNoCtx};
+    }
+};
+
+static StaticSingleShotUser *s_staticSingleShotUser = nullptr;
+
+void tst_QTimer::singleShot_static()
+{
+    QCoreApplication::processEvents();
+    QCOMPARE(s_staticSingleShotUser->helper.calls, s_staticSingleShotUser->calls());
+}
+
+// NOTE: to prevent any static initialization order fiasco, we handle QTEST_MAIN
+//       ourselves, but instantiate the staticSingleShotUser before qApp
+
+int main(int argc, char *argv[])
+{
+    StaticSingleShotUser staticSingleShotUser;
+    s_staticSingleShotUser = &staticSingleShotUser;
+    QTEST_MAIN_IMPL(tst_QTimer)
+}
+
 #include "tst_qtimer.moc"
diff --git a/tests/benchmarks/corelib/kernel/qtimer_vs_qmetaobject/tst_qtimer_vs_qmetaobject.cpp b/tests/benchmarks/corelib/kernel/qtimer_vs_qmetaobject/tst_qtimer_vs_qmetaobject.cpp
index 07976aa056..6af5b8d586 100644
--- a/tests/benchmarks/corelib/kernel/qtimer_vs_qmetaobject/tst_qtimer_vs_qmetaobject.cpp
+++ b/tests/benchmarks/corelib/kernel/qtimer_vs_qmetaobject/tst_qtimer_vs_qmetaobject.cpp
@@ -28,6 +28,7 @@
 
 #include <QtCore>
 #include <QtTest/QtTest>
+#include <QThread>
 
 #define INVOKE_COUNT 10000
 
@@ -37,6 +38,8 @@ class qtimer_vs_qmetaobject : public QObject
 private slots:
     void bench();
     void bench_data();
+    void benchBackgroundThread();
+    void benchBackgroundThread_data() { bench_data(); }
 };
 
 class InvokeCounter : public QObject {
@@ -47,8 +50,10 @@ public slots:
     void invokeSlot() {
         count++;
         if (count == INVOKE_COUNT)
-            QTestEventLoop::instance().exitLoop();
+            emit allInvoked();
     }
+signals:
+    void allInvoked();
 protected:
     int count;
 };
@@ -98,11 +103,11 @@ void qtimer_vs_qmetaobject::bench()
 
     QBENCHMARK {
         InvokeCounter invokeCounter;
+        QSignalSpy spy(&invokeCounter, &InvokeCounter::allInvoked);
         for(int i = 0; i < INVOKE_COUNT; ++i) {
             invoke(&invokeCounter);
         }
-        QTestEventLoop::instance().enterLoop(10);
-        QVERIFY(!QTestEventLoop::instance().timeout());
+        QVERIFY(spy.wait(10000));
     }
 }
 
@@ -118,6 +123,16 @@ void qtimer_vs_qmetaobject::bench_data()
     QTest::addRow("invokeMethod_functor") << 6;
 }
 
+void qtimer_vs_qmetaobject::benchBackgroundThread()
+{
+#if !QT_CONFIG(cxx11_future)
+    QSKIP("This test requires QThread::create");
+#else
+    QScopedPointer<QThread> thread(QThread::create([this]() { bench(); }));
+    thread->start();
+    QVERIFY(thread->wait());
+#endif
+}
 
 QTEST_MAIN(qtimer_vs_qmetaobject)