MapReduce refactoring and improvements

- QThread instead of std::thread for better integration with Qt
- Use thread pool for recycling threads
- Map and reduce functions are handled like any function passed
  to runAsync, so they either report results through the
  QFutureInterface, or through the return value.
- Automatically deduce the reduce result type

Change-Id: I7a31370c21f8c27b378cd87c3d5974b162449ce1
Reviewed-by: Tobias Hunger <tobias.hunger@theqtcompany.com>

src/libs/utils/filesearch.cpp

@@ -83,8 +83,8 @@ class FileSearch
 public:
     FileSearch(const QString &searchTerm, QTextDocument::FindFlags flags,
                QMap<QString, QString> fileToContentsMap);
-    FileSearchResultList operator()(QFutureInterface<FileSearchResultList> futureInterface,
-                                          const FileIterator::Item &item) const;
+    void operator()(QFutureInterface<FileSearchResultList> &futureInterface,
+                    const FileIterator::Item &item) const;
 
 private:
     QMap<QString, QString> fileToContentsMap;
@@ -104,8 +104,8 @@ public:
     FileSearchRegExp(const QString &searchTerm, QTextDocument::FindFlags flags,
                      QMap<QString, QString> fileToContentsMap);
     FileSearchRegExp(const FileSearchRegExp &other);
-    FileSearchResultList operator()(QFutureInterface<FileSearchResultList> futureInterface,
-                                          const FileIterator::Item &item) const;
+    void operator()(QFutureInterface<FileSearchResultList> &futureInterface,
+                    const FileIterator::Item &item) const;
 
 private:
     QRegularExpressionMatch doGuardedMatch(const QString &line, int offset) const;
@@ -129,17 +129,21 @@ FileSearch::FileSearch(const QString &searchTerm, QTextDocument::FindFlags flags
     termDataUpper = searchTermUpper.constData();
 }
 
-FileSearchResultList FileSearch::operator()(QFutureInterface<FileSearchResultList> futureInterface,
-                                                  const FileIterator::Item &item) const
+void FileSearch::operator()(QFutureInterface<FileSearchResultList> &futureInterface,
+                            const FileIterator::Item &item) const
 {
-    FileSearchResultList results;
     if (futureInterface.isCanceled())
-        return results;
+        return;
+    futureInterface.setProgressRange(0, 1);
+    futureInterface.setProgressValue(0);
+    FileSearchResultList results;
     QFile file;
     QTextStream stream;
     QString tempString;
-    if (!openStream(item.filePath, item.encoding, &stream, &file, &tempString, fileToContentsMap))
-        return results;
+    if (!openStream(item.filePath, item.encoding, &stream, &file, &tempString, fileToContentsMap)) {
+        futureInterface.cancel(); // failure
+        return;
+    }
     int lineNr = 0;
 
     while (!stream.atEnd()) {
@@ -211,7 +215,10 @@ FileSearchResultList FileSearch::operator()(QFutureInterface<FileSearchResultLis
     }
     if (file.isOpen())
         file.close();
-    return results;
+    if (!futureInterface.isCanceled()) {
+        futureInterface.reportResult(results);
+        futureInterface.setProgressValue(1);
+    }
 }
 
 FileSearchRegExp::FileSearchRegExp(const QString &searchTerm, QTextDocument::FindFlags flags,
@@ -238,17 +245,21 @@ QRegularExpressionMatch FileSearchRegExp::doGuardedMatch(const QString &line, in
     return expression.match(line, offset);
 }
 
-FileSearchResultList FileSearchRegExp::operator()(QFutureInterface<FileSearchResultList> futureInterface,
-                                                        const FileIterator::Item &item) const
+void FileSearchRegExp::operator()(QFutureInterface<FileSearchResultList> &futureInterface,
+                                  const FileIterator::Item &item) const
 {
-    FileSearchResultList results;
     if (futureInterface.isCanceled())
-        return results;
+        return;
+    futureInterface.setProgressRange(0, 1);
+    futureInterface.setProgressValue(0);
+    FileSearchResultList results;
     QFile file;
     QTextStream stream;
     QString tempString;
-    if (!openStream(item.filePath, item.encoding, &stream, &file, &tempString, fileToContentsMap))
-        return results;
+    if (!openStream(item.filePath, item.encoding, &stream, &file, &tempString, fileToContentsMap)) {
+        futureInterface.cancel(); // failure
+        return;
+    }
     int lineNr = 0;
 
     QString line;
@@ -277,7 +288,10 @@ FileSearchResultList FileSearchRegExp::operator()(QFutureInterface<FileSearchRes
     }
     if (file.isOpen())
         file.close();
-    return results;
+    if (!futureInterface.isCanceled()) {
+        futureInterface.reportResult(results);
+        futureInterface.setProgressValue(1);
+    }
 }
 
 struct SearchState
@@ -345,7 +359,7 @@ void cleanUpFileSearch(QFutureInterface<FileSearchResultList> &futureInterface,
 QFuture<FileSearchResultList> Utils::findInFiles(const QString &searchTerm, FileIterator *files,
     QTextDocument::FindFlags flags, QMap<QString, QString> fileToContentsMap)
 {
-    return mapReduce<FileSearchResultList>(std::cref(*files),
+    return mapReduce(std::cref(*files),
                      [searchTerm, files](QFutureInterface<FileSearchResultList> &futureInterface) {
                          return initFileSearch(futureInterface, searchTerm, files);
                      },
@@ -357,7 +371,7 @@ QFuture<FileSearchResultList> Utils::findInFiles(const QString &searchTerm, File
 QFuture<FileSearchResultList> Utils::findInFilesRegExp(const QString &searchTerm, FileIterator *files,
     QTextDocument::FindFlags flags, QMap<QString, QString> fileToContentsMap)
 {
-    return mapReduce<FileSearchResultList>(std::cref(*files),
+    return mapReduce(std::cref(*files),
                      [searchTerm, files](QFutureInterface<FileSearchResultList> &futureInterface) {
                          return initFileSearch(futureInterface, searchTerm, files);
                      },

src/libs/utils/runextensions.h

@@ -222,12 +222,9 @@ private:
 
 // void function that does not take QFutureInterface
 template <typename ResultType, typename Function, typename... Args>
-void runAsyncReturnVoidDispatch(std::true_type, QFutureInterface<ResultType> futureInterface, Function &&function, Args&&... args)
+void runAsyncReturnVoidDispatch(std::true_type, QFutureInterface<ResultType>, Function &&function, Args&&... args)
 {
     function(std::forward<Args>(args)...);
-    if (futureInterface.isPaused())
-        futureInterface.waitForResume();
-    futureInterface.reportFinished();
 }
 
 // non-void function that does not take QFutureInterface
@@ -235,9 +232,6 @@ template <typename ResultType, typename Function, typename... Args>
 void runAsyncReturnVoidDispatch(std::false_type, QFutureInterface<ResultType> futureInterface, Function &&function, Args&&... args)
 {
     futureInterface.reportResult(function(std::forward<Args>(args)...));
-    if (futureInterface.isPaused())
-        futureInterface.waitForResume();
-    futureInterface.reportFinished();
 }
 
 // function that takes QFutureInterface
@@ -245,9 +239,6 @@ template <typename ResultType, typename Function, typename... Args>
 void runAsyncQFutureInterfaceDispatch(std::true_type, QFutureInterface<ResultType> futureInterface, Function &&function, Args&&... args)
 {
     function(futureInterface, std::forward<Args>(args)...);
-    if (futureInterface.isPaused())
-        futureInterface.waitForResume();
-    futureInterface.reportFinished();
 }
 
 // function that does not take QFutureInterface
@@ -374,6 +365,9 @@ private:
     {
         // invalidates data, which is moved into the call
         runAsyncImpl(futureInterface, std::move(std::get<index>(data))...);
+        if (futureInterface.isPaused())
+            futureInterface.waitForResume();
+        futureInterface.reportFinished();
     }
 
     Data data;

tests/auto/auto.pro

@@ -17,6 +17,7 @@ SUBDIRS += \
     json \
     utils \
     filesearch \
+    mapreduce \
     runextensions \
     sdktool \
     valgrind

tests/auto/mapreduce/mapreduce.pro

+QTC_LIB_DEPENDS += utils
+include(../qttest.pri)
+
+# Input
+SOURCES += tst_mapreduce.cpp
+HEADERS += $$IDE_SOURCE_TREE/src/libs/utils/mapreduce.h

tests/auto/mapreduce/mapreduce.qbs

+import qbs
+
+QtcAutotest {
+    name: "Map reduce autotest"
+    Depends { name: "Utils" }
+
+    files: [
+        "tst_mapreduce.cpp",
+    ]
+}

tests/auto/mapreduce/tst_mapreduce.cpp

+/****************************************************************************
+**
+** Copyright (C) 2016 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+****************************************************************************/
+
+#include <utils/algorithm.h>
+#include <utils/mapreduce.h>
+
+#include <QtTest>
+
+class tst_MapReduce : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void mapReduce();
+};
+
+static int returnxx(int x)
+{
+    return x*x;
+}
+
+static void returnxxThroughFutureInterface(QFutureInterface<int> &fi, int x)
+{
+    fi.reportResult(x*x);
+}
+
+void tst_MapReduce::mapReduce()
+{
+    const auto dummyInit = [](QFutureInterface<double>& fi) -> double {
+        fi.reportResult(0.);
+        return 0.;
+    };
+    const auto reduceWithFutureInterface = [](QFutureInterface<double>& fi, double &state, int value) {
+        state += value;
+        fi.reportResult(value);
+    };
+    const auto reduceWithReturn = [](double &state, int value) -> double {
+        state += value;
+        return value;
+    };
+    const auto cleanupHalfState = [](QFutureInterface<double> &fi, double &state) {
+        state /= 2.;
+        fi.reportResult(state);
+    };
+
+    // TODO: cannot use function returnxx without pointer here because of decayCopy of arguments in runAsync
+    {
+        QList<double> results = Utils::mapReduce(QList<int>({1, 2, 3, 4, 5}),
+                                                 dummyInit, &returnxx,
+                                                 reduceWithFutureInterface, cleanupHalfState)
+                .results();
+        Utils::sort(results); // mapping order is undefined
+        QCOMPARE(results, QList<double>({0., 1., 4., 9., 16., 25., 27.5}));
+    }
+    {
+        QList<double> results = Utils::mapReduce(QList<int>({1, 2, 3, 4, 5}),
+                                                 dummyInit, &returnxxThroughFutureInterface,
+                                                 reduceWithFutureInterface, cleanupHalfState)
+                .results();
+        Utils::sort(results); // mapping order is undefined
+        QCOMPARE(results, QList<double>({0., 1., 4., 9., 16., 25., 27.5}));
+    }
+    {
+        QList<double> results = Utils::mapReduce(QList<int>({1, 2, 3, 4, 5}),
+                                                 dummyInit, &returnxx,
+                                                 reduceWithReturn, cleanupHalfState)
+                .results();
+        Utils::sort(results); // mapping order is undefined
+        QCOMPARE(results, QList<double>({0., 1., 4., 9., 16., 25., 27.5}));
+    }
+}
+
+QTEST_MAIN(tst_MapReduce)
+
+#include "tst_mapreduce.moc"