From a0faf9e23666d4aa26a93d6e9ebf420e71d5e6c2 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Tue, 21 Nov 2017 10:26:35 +0100
Subject: 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>
---
 src/corelib/thread/qthread_p.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'src/corelib/thread/qthread_p.h')

diff --git a/src/corelib/thread/qthread_p.h b/src/corelib/thread/qthread_p.h
index 1d38eb0ebf..baeefd87ff 100644
--- a/src/corelib/thread/qthread_p.h
+++ b/src/corelib/thread/qthread_p.h
@@ -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;
-- 
cgit v1.2.3