From dfd07205e58d67324a82e5aed0ce7fec63bd9368 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Wed, 12 Jul 2023 17:32:25 +0200 Subject: [PATCH] QFuture: Extract Method watchContinuation() (DRY && SCARY) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old code violated the following principles: - DRY: the same code occurred 3× in the code-base - SCARY: the vast majority doesn't actually depend on template arguments, causing template bloat Solve both with a tiered Extract Method. We cannot change the order of the operations performed on QBasicFutureWatcher, in particular not the connect() to the contination w.r.t. setFuture(), so we cannot leave the connect to the continuation lambda outside the function, as it would mean to also leave the setFuture() call outside. Thanks to Volker's makeCallableObject(), we can, however, type-erase the lambda using QSlotObjectBase, which is what connect() internally creates, anyway, therefore bringing the whole function behind the ABI boundary. As a non-QObject, non-QMetaObject friend, we're lacking support for actually doing something useful with a QSlotObjectBase, but that can be fixed in the implementation now. The interface is stable, which is what matters for 6.6 now. This will allow a subsequent commit to drag QBasicFutureWatcher behind the ABI boundary, unexporting it. Saves a whopping 8KiB in tst_qfuture text size on optimized C++20 Linux AMD64 GCC9 builds. Pick-to: 6.6 Done-with: Fabian Kosmale Change-Id: I0e5c2564907d92f6938689ab249be11fc0332ba5 Reviewed-by: Qt CI Bot Reviewed-by: Arno Rehn Reviewed-by: Marc Mutz Reviewed-by: Volker Hilsheimer --- src/corelib/thread/qfuture_impl.h | 37 ++++++++++--------------- src/corelib/thread/qfutureinterface.cpp | 32 +++++++++++++++++++++ 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/src/corelib/thread/qfuture_impl.h b/src/corelib/thread/qfuture_impl.h index 5b4c929a36..d8fe946b92 100644 --- a/src/corelib/thread/qfuture_impl.h +++ b/src/corelib/thread/qfuture_impl.h @@ -590,6 +590,18 @@ void Continuation::create(F &&func, f->d.setContinuation(ContinuationWrapper(std::move(continuation)), fi.d); } +// defined in qfutureinterface.cpp: +Q_CORE_EXPORT void watchContinuationImpl(const QObject *context, QSlotObjectBase *slotObj, + QFutureInterfaceBase &fi); +template +void watchContinuation(const QObject *context, Continuation &&c, QFutureInterfaceBase &fi) +{ + using Prototype = typename QtPrivate::Callable::Function; + watchContinuationImpl(context, + QtPrivate::makeCallableObject(std::forward(c)), + fi); +} + template template void Continuation::create(F &&func, @@ -610,15 +622,7 @@ void Continuation::create(F &&func, continuationJob.execute(); }; - auto *watcher = new QBasicFutureWatcher; - watcher->moveToThread(context->thread()); - QObject::connect(watcher, &QBasicFutureWatcher::finished, - context, std::move(continuation)); - QObject::connect(watcher, &QBasicFutureWatcher::finished, - watcher, &QObject::deleteLater); - QObject::connect(context, &QObject::destroyed, - watcher, &QObject::deleteLater); - watcher->setFuture(f->d); + QtPrivate::watchContinuation(context, std::move(continuation), f->d); } template @@ -710,12 +714,7 @@ void FailureHandler::create(F &&function, QFuturemoveToThread(context->thread()); - QObject::connect(watcher, &QBasicFutureWatcher::finished, context, std::move(failureContinuation)); - QObject::connect(watcher, &QBasicFutureWatcher::finished, watcher, &QObject::deleteLater); - QObject::connect(context, &QObject::destroyed, watcher, &QObject::deleteLater); - watcher->setFuture(future->d); + QtPrivate::watchContinuation(context, std::move(failureContinuation), future->d); } template @@ -809,13 +808,7 @@ public: run(std::forward(handler), parentFuture, std::move(promise)); }; - auto *watcher = new QBasicFutureWatcher; - watcher->moveToThread(context->thread()); - QObject::connect(watcher, &QBasicFutureWatcher::finished, - context, std::move(canceledContinuation)); - QObject::connect(watcher, &QBasicFutureWatcher::finished, watcher, &QObject::deleteLater); - QObject::connect(context, &QObject::destroyed, watcher, &QObject::deleteLater); - watcher->setFuture(future->d); + QtPrivate::watchContinuation(context, std::move(canceledContinuation), future->d); } template diff --git a/src/corelib/thread/qfutureinterface.cpp b/src/corelib/thread/qfutureinterface.cpp index 8f73dde253..de936005c3 100644 --- a/src/corelib/thread/qfutureinterface.cpp +++ b/src/corelib/thread/qfutureinterface.cpp @@ -4,12 +4,14 @@ // qfutureinterface.h included from qfuture.h #include "qfuture.h" #include "qfutureinterface_p.h" +#include "qbasicfuturewatcher.h" #include #include #include #include // for qYieldCpu() #include +#include #ifdef interface # undef interface @@ -43,6 +45,36 @@ const auto suspendingOrSuspended = } // unnamed namespace +void QtPrivate::watchContinuationImpl(const QObject *context, QSlotObjectBase *slotObj, + QFutureInterfaceBase &fi) +{ + Q_ASSERT(context); + Q_ASSERT(slotObj); + + // ### we're missing `QSlotObjectPtr`... + struct Deleter { + void operator()(QSlotObjectBase *p) const { p->destroyIfLastRef(); } + }; + auto slot = std::unique_ptr(slotObj); + + auto *watcher = new QBasicFutureWatcher; + watcher->moveToThread(context->thread()); + // ### we're missing a convenient way to `QObject::connect()` to a `QSlotObjectBase`... + QObject::connect(watcher, &QBasicFutureWatcher::finished, + // for the following, cf. QMetaObject::invokeMethodImpl(): + // we know `slot` is a lambda returning `void`, so we can just + // `call()` with `obj` and `args[0]` set to `nullptr`: + watcher, [slot = std::move(slot)] { + void *args[] = { nullptr }; // for `void` return value + slot->call(nullptr, args); + }); + QObject::connect(watcher, &QBasicFutureWatcher::finished, + watcher, &QObject::deleteLater); + QObject::connect(context, &QObject::destroyed, + watcher, &QObject::deleteLater); + watcher->setFuture(fi); +} + QFutureCallOutInterface::~QFutureCallOutInterface() = default;