From bfd2d301693bbf278157db23dd322d30d66edd3b Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Mon, 4 Apr 2022 18:22:46 +0200
Subject: [PATCH] WhenAllContext: optimize atomic operations

The old code performed one load-acquire (in the assertion) and one
ordered(!) fetch_sub() (in the pre-decrement operator) where one
relaxed fetch_sub() would suffice (it's just a counter).

Fix by caching the result of the relaxed fetch_sub() and performing
the following checks on the cached result.

As drive-bys, rename the atomic member variable (to catch any outside
uses) and make the ctor explicit.

Found by locally disabling the QAtomic<T> -> T implicit conversion
operators.

Change-Id: Ifdc11c2c4807b71f4cab2ba9f5405ace7d8d71a9
Reviewed-by: Sona Kurazyan <sona.kurazyan@qt.io>
---
 src/corelib/thread/qfuture_impl.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/corelib/thread/qfuture_impl.h b/src/corelib/thread/qfuture_impl.h
index 90abb99d8e8..e4b0ea62894 100644
--- a/src/corelib/thread/qfuture_impl.h
+++ b/src/corelib/thread/qfuture_impl.h
@@ -1003,20 +1003,21 @@ struct WhenAllContext
 {
     using ValueType = typename ResultFutures::value_type;
 
-    WhenAllContext(qsizetype size) : count(size) {}
+    explicit WhenAllContext(qsizetype size) : remaining(size) {}
 
     template<typename T = ValueType>
     void checkForCompletion(qsizetype index, T &&future)
     {
         futures[index] = std::forward<T>(future);
-        Q_ASSERT(count > 0);
-        if (--count <= 0) {
+        const auto oldRemaining = remaining.fetchAndSubRelaxed(1);
+        Q_ASSERT(oldRemaining > 0);
+        if (oldRemaining <= 1) { // that was the last one
             promise.addResult(futures);
             promise.finish();
         }
     }
 
-    QAtomicInteger<qsizetype> count;
+    QAtomicInteger<qsizetype> remaining;
     QPromise<ResultFutures> promise;
     ResultFutures futures;
 };