Optimize QThread::isInterruptionRequested()
To signal a thread to cancel, nothing more than a std::atomic_flag is needed, but the implementation actually used mutexes, and weird run-state introspection, so we can't just swap it out for a std::atomic_flag. Instead, we retain the principal logic, however weird it is, and just optimize the common case where isInterruptionRequested() is called from the secondary thread, repeatedly. We add a fast-path that just checks that d->interruptionRequested is not set. That requires nothing more than a relaxed atomic load, because there's no new value read that could be used as a signal to the secondary thread that some condition changed. "What signal?", you may ask. Well, one can think of users doing this: void cancel() { m_why = tr("&Canceled"); requestIterruption(); } void run() override { while (!isInterruptionRequested()) { doWork(); } emit progress(100, 100, m_why); } We need to keep this code working, at least until Qt 6. But the code can already now only rely on synchronization if isInterruptionRequested() returns true. If it returns false, then requestInterruption() has not been called, yet, and any modifications done prior to the requestInterruption() call are not visible in the secondary thead. So we still lock the mutex, and in general don't change the semantics of the functions, except that we don't lock the mutex in the case where the flag wasn't set in the first place. This makes calling isInterruptionRequested() as cheap as it can get, assuming a lock-free implementation, of course. I opted to use a std::atomic<bool> instead of QAtomicInt, as the latter does not have loadRelaxed()/storeRelaxed(), and because it future-proofs the code. Change-Id: I67faf36b8de73d2723f9cdd66c416010d0873d98 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com> Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
This commit is contained in:
parent
1c0fcbc887
commit
a0faf9e236
@ -849,10 +849,12 @@ void QThread::requestInterruption()
|
||||
return;
|
||||
}
|
||||
Q_D(QThread);
|
||||
// ### Qt 6: use std::atomic_flag, and document that
|
||||
// requestInterruption/isInterruptionRequested do not synchronize with each other
|
||||
QMutexLocker locker(&d->mutex);
|
||||
if (!d->running || d->finished || d->isInFinish)
|
||||
return;
|
||||
d->interruptionRequested = true;
|
||||
d->interruptionRequested.store(true, std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
/*!
|
||||
@ -881,10 +883,12 @@ void QThread::requestInterruption()
|
||||
bool QThread::isInterruptionRequested() const
|
||||
{
|
||||
Q_D(const QThread);
|
||||
QMutexLocker locker(&d->mutex);
|
||||
if (!d->running || d->finished || d->isInFinish)
|
||||
// fast path: check that the flag is not set:
|
||||
if (!d->interruptionRequested.load(std::memory_order_relaxed))
|
||||
return false;
|
||||
return d->interruptionRequested;
|
||||
// slow path: if the flag is set, take into account run status:
|
||||
QMutexLocker locker(&d->mutex);
|
||||
return d->running && !d->finished && !d->isInFinish;
|
||||
}
|
||||
|
||||
/*
|
||||
|
@ -63,6 +63,7 @@
|
||||
#include "private/qobject_p.h"
|
||||
|
||||
#include <algorithm>
|
||||
#include <atomic>
|
||||
|
||||
#ifdef Q_OS_WINRT
|
||||
namespace ABI {
|
||||
@ -165,7 +166,7 @@ public:
|
||||
bool running;
|
||||
bool finished;
|
||||
bool isInFinish; //when in QThreadPrivate::finish
|
||||
bool interruptionRequested;
|
||||
std::atomic<bool> interruptionRequested;
|
||||
|
||||
bool exited;
|
||||
int returnCode;
|
||||
|
Loading…
Reference in New Issue
Block a user