From 699162c6fa6121cc496338f1d8d6e1b4287f7760 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Tue, 16 Jul 2024 12:38:33 +0200 Subject: [PATCH] Rework QtPrivate::Continuation to get rid of virtual calls and rename it The Continuation class is a template and polymorphic, which likely means that it instantiates a new type with a vtable for each combination of template parameters. Remove the Sync/Async continuation classes to get rid of the hierarcy. Instead, encode all the information about the type of the continuation into the class itself. This requires to add two new members: * QThreadPool (which was previously a member of an async class) * QRunnable (async class was previously derived from it). We cannot really eliminate the need for QThreadPool and QRunnable without a larger refactoring of the entire QFuture framework, because QFutureInterface requires a QRunnable in its implementation. However, this patch already allows us to get rid of all the virtual functions in the Continuation implementation. While on it, also remove the Function member and derive from QtPrivate::CompactStorage instead. This results in a significant reduce of a binary size for the user projects, specially if they have a wide usage of continuations. Taking tst_qfuture as an extreme example, the binary size on a release build on macOS is reduced by ~15% - from 3723576 to 3156344 bytes. The change seems to be BC, because all Continuation usages are from the inline code in other template classes, so most probably a binary linked with the older version of Qt will simply keep using the old implementation. Still, if the user project creates a shared library that utilizes continuations, and builds the library with the default visibility attributes, the private Continuation class gets exposed in the exported methods. To avoid ODR violations in such cases, simply rename the class to CompactContinuation. Fixes: QTBUG-124909 Change-Id: I8c8263dbaf407d922aed2017d659bbc6fd87083d Reviewed-by: Thiago Macieira --- src/corelib/thread/qfuture.h | 8 +- src/corelib/thread/qfuture_impl.h | 144 ++++++++++++-------------- src/corelib/thread/qfutureinterface.h | 4 +- 3 files changed, 71 insertions(+), 85 deletions(-) diff --git a/src/corelib/thread/qfuture.h b/src/corelib/thread/qfuture.h index 3945df066aa..4aacd97f233 100644 --- a/src/corelib/thread/qfuture.h +++ b/src/corelib/thread/qfuture.h @@ -280,7 +280,7 @@ private: friend class QFutureInterfaceBase; template - friend class QtPrivate::Continuation; + friend class QtPrivate::CompactContinuation; template friend class QtPrivate::CanceledHandler; @@ -339,7 +339,7 @@ QFuture::template ResultType> QFuture::then(QtFuture::Launch policy, Function &&function) { QFutureInterface> promise(QFutureInterfaceBase::State::Pending); - QtPrivate::Continuation, ResultType, T>::create( + QtPrivate::CompactContinuation, ResultType, T>::create( std::forward(function), this, promise, policy); return promise.future(); } @@ -350,7 +350,7 @@ QFuture::template ResultType> QFuture::then(QTh Function &&function) { QFutureInterface> promise(QFutureInterfaceBase::State::Pending); - QtPrivate::Continuation, ResultType, T>::create( + QtPrivate::CompactContinuation, ResultType, T>::create( std::forward(function), this, promise, pool); return promise.future(); } @@ -361,7 +361,7 @@ QFuture::template ResultType> QFuture::then(QOb Function &&function) { QFutureInterface> promise(QFutureInterfaceBase::State::Pending); - QtPrivate::Continuation, ResultType, T>::create( + QtPrivate::CompactContinuation, ResultType, T>::create( std::forward(function), this, promise, context); return promise.future(); } diff --git a/src/corelib/thread/qfuture_impl.h b/src/corelib/thread/qfuture_impl.h index 351093adc7b..780e39dd8cb 100644 --- a/src/corelib/thread/qfuture_impl.h +++ b/src/corelib/thread/qfuture_impl.h @@ -11,6 +11,7 @@ #endif #include +#include #include #include #include @@ -285,19 +286,37 @@ using IsForwardIterable = std::forward_iterator_tag>; template -class Continuation +class CompactContinuation : private CompactStorage { - Q_DISABLE_COPY_MOVE(Continuation) + Q_DISABLE_COPY_MOVE(CompactContinuation) public: + using Storage = CompactStorage; + template - Continuation(F &&func, const QFuture &f, QPromise &&p) - : promise(std::move(p)), parentFuture(f), function(std::forward(func)) + CompactContinuation(F &&func, const QFuture &f, QPromise &&p) + : Storage{std::forward(func)}, promise(std::move(p)), parentFuture(f), type(Type::Sync) { } - virtual ~Continuation() = default; + + template + CompactContinuation(F &&func, const QFuture &f, QPromise &&p, + QThreadPool *pool) + : Storage{std::forward(func)}, promise(std::move(p)), parentFuture(f), + threadPool(pool), type(Type::Async) + { + runObj = QRunnable::create([this] { + this->runFunction(); + delete this; + }); + runObj->setAutoDelete(false); + } + + ~CompactContinuation() { delete runObj; } bool execute(); + QRunnable *runnable() const { return runObj; } + template static void create(F &&func, QFuture *f, QFutureInterface &fi, QtFuture::Launch policy); @@ -319,63 +338,30 @@ private: void fulfillPromise(Args &&... args); protected: - virtual void runImpl() = 0; + void runImpl() + { + if (type == Type::Sync) { + runFunction(); + } else { + Q_ASSERT(runObj); + QThreadPool *pool = threadPool ? threadPool : QThreadPool::globalInstance(); + pool->start(runObj); + } + } void runFunction(); protected: + enum class Type : quint8 { + Sync, + Async + }; + QPromise promise; QFuture parentFuture; - Function function; -}; - -template -class SyncContinuation final : public Continuation -{ -public: - template - SyncContinuation(F &&func, const QFuture &f, QPromise &&p) - : Continuation(std::forward(func), f, - std::move(p)) - { - } - - ~SyncContinuation() override = default; - -private: - void runImpl() override { this->runFunction(); } -}; - -template -class AsyncContinuation final : public QRunnable, - public Continuation -{ -public: - template - AsyncContinuation(F &&func, const QFuture &f, QPromise &&p, - QThreadPool *pool = nullptr) - : Continuation(std::forward(func), f, - std::move(p)), - threadPool(pool) - { - } - - ~AsyncContinuation() override = default; - -private: - void runImpl() override // from Continuation - { - QThreadPool *pool = threadPool ? threadPool : QThreadPool::globalInstance(); - pool->start(this); - } - - void run() override // from QRunnable - { - this->runFunction(); - } - -private: - QThreadPool *threadPool; + QThreadPool *threadPool = nullptr; + QRunnable *runObj = nullptr; + Type type; }; #ifndef QT_NO_EXCEPTIONS @@ -415,7 +401,7 @@ private: #endif template -void Continuation::runFunction() +void CompactContinuation::runFunction() { promise.start(); @@ -439,9 +425,9 @@ void Continuation::runFunction() } else { if constexpr (std::is_void_v) { if constexpr (std::is_invocable_v>) - function(parentFuture); + this->object()(parentFuture); else - function(); + this->object()(); } else if constexpr (std::is_invocable_v) { fulfillVoidPromise(); } else { @@ -449,7 +435,7 @@ void Continuation::runFunction() // that nothing unexpected happened. static_assert(std::is_invocable_v>, "The continuation is not invocable with the provided arguments"); - function(parentFuture); + this->object()(parentFuture); } } #ifndef QT_NO_EXCEPTIONS @@ -461,7 +447,7 @@ void Continuation::runFunction() } template -bool Continuation::execute() +bool CompactContinuation::execute() { Q_ASSERT(parentFuture.isFinished()); @@ -513,7 +499,7 @@ private: template template -void Continuation::create(F &&func, +void CompactContinuation::create(F &&func, QFuture *f, QFutureInterface &fi, QtFuture::Launch policy) @@ -538,20 +524,20 @@ void Continuation::create(F &&func, auto continuation = [func = std::forward(func), fi, promise_ = QPromise(fi), pool, launchAsync](const QFutureInterfaceBase &parentData) mutable { const auto parent = QFutureInterface(parentData).future(); - Continuation *continuationJob = nullptr; + CompactContinuation *continuationJob = nullptr; if (launchAsync) { - auto asyncJob = new AsyncContinuation( + auto asyncJob = new CompactContinuation( std::forward(func), parent, std::move(promise_), pool); - fi.setRunnable(asyncJob); + fi.setRunnable(asyncJob->runnable()); continuationJob = asyncJob; } else { - continuationJob = new SyncContinuation( + continuationJob = new CompactContinuation( std::forward(func), parent, std::move(promise_)); } bool isLaunched = continuationJob->execute(); // If continuation is successfully launched, AsyncContinuation will be deleted - // by the QThreadPool which has started it. Synchronous continuation will be + // from the QRunnable's lambda. Synchronous continuation will be // executed immediately, so it's safe to always delete it here. if (!(launchAsync && isLaunched)) { delete continuationJob; @@ -563,7 +549,7 @@ void Continuation::create(F &&func, template template -void Continuation::create(F &&func, +void CompactContinuation::create(F &&func, QFuture *f, QFutureInterface &fi, QThreadPool *pool) @@ -576,11 +562,11 @@ void Continuation::create(F &&func, auto continuation = [func = std::forward(func), promise_ = QPromise(fi), pool](const QFutureInterfaceBase &parentData) mutable { const auto parent = QFutureInterface(parentData).future(); - auto continuationJob = new AsyncContinuation( + auto continuationJob = new CompactContinuation( std::forward(func), parent, std::move(promise_), pool); bool isLaunched = continuationJob->execute(); // If continuation is successfully launched, AsyncContinuation will be deleted - // by the QThreadPool which has started it. + // from the QRunnable's lambda. if (!isLaunched) { delete continuationJob; continuationJob = nullptr; @@ -600,7 +586,7 @@ void watchContinuation(const QObject *context, Continuation &&c, QFutureInterfac template template -void Continuation::create(F &&func, +void CompactContinuation::create(F &&func, QFuture *f, QFutureInterface &fi, QObject *context) @@ -613,7 +599,7 @@ void Continuation::create(F &&func, // destroyed and, if it is not yet finished, cancelled. auto continuation = [func = std::forward(func), parent = *f, promise_ = QPromise(fi)]() mutable { - SyncContinuation continuationJob( + CompactContinuation continuationJob( std::forward(func), parent, std::move(promise_)); continuationJob.execute(); }; @@ -622,7 +608,7 @@ void Continuation::create(F &&func, } template -void Continuation::fulfillPromiseWithResult() +void CompactContinuation::fulfillPromiseWithResult() { if constexpr (std::is_copy_constructible_v) fulfillPromise(parentFuture.result()); @@ -631,16 +617,16 @@ void Continuation::fulfillPromiseWithRes } template -void Continuation::fulfillVoidPromise() +void CompactContinuation::fulfillVoidPromise() { if constexpr (std::is_copy_constructible_v) - function(parentFuture.result()); + this->object()(parentFuture.result()); else - function(parentFuture.takeResult()); + this->object()(parentFuture.takeResult()); } template -void Continuation::fulfillPromiseWithVoidResult() +void CompactContinuation::fulfillPromiseWithVoidResult() { if constexpr (std::is_invocable_v>) fulfillPromise(parentFuture); @@ -650,9 +636,9 @@ void Continuation::fulfillPromiseWithVoi template template -void Continuation::fulfillPromise(Args &&... args) +void CompactContinuation::fulfillPromise(Args &&... args) { - promise.addResult(std::invoke(function, std::forward(args)...)); + promise.addResult(std::invoke(this->object(), std::forward(args)...)); } template diff --git a/src/corelib/thread/qfutureinterface.h b/src/corelib/thread/qfutureinterface.h index ea7d40ad907..5e28cecda1d 100644 --- a/src/corelib/thread/qfutureinterface.h +++ b/src/corelib/thread/qfutureinterface.h @@ -28,7 +28,7 @@ class QFutureWatcherBasePrivate; namespace QtPrivate { template -class Continuation; +class CompactContinuation; class ExceptionStore; @@ -171,7 +171,7 @@ private: friend class QFutureWatcherBasePrivate; template - friend class QtPrivate::Continuation; + friend class QtPrivate::CompactContinuation; template friend class QtPrivate::CanceledHandler;