File search: Avoid use of QtConcurrent

QtConcurrent limits resource usage to a global number of simultaneous
threads. That means that if some QtConcurrent based algorithm currently
grabs all threads, any other use of QtConcurrent blocks, which is not
what we want.
Use the new threading methods of C++11 instead, but still use
QFuture(Interface) manually for the progress, result and status
reporting.

Task-number: QTCREATORBUG-14640
Change-Id: I6379d2f2a01b6d200811ef4be0bbfcd4493dd154
Reviewed-by: Orgad Shaneh <orgads@gmail.com>

src/libs/utils/filesearch.cpp

@@ -35,7 +35,6 @@
 #include <QMutex>
 #include <QRegularExpression>
 #include <QTextCodec>
-#include <QtConcurrentMap>
 
 #include <cctype>
 
@@ -88,13 +87,12 @@ class FileSearch
 {
 public:
     FileSearch(const QString &searchTerm, QTextDocument::FindFlags flags,
-               QMap<QString, QString> fileToContentsMap,
-               QFutureInterface<FileSearchResultList> *futureInterface);
-    const FileSearchResultList operator()(const FileIterator::Item &item) const;
+               QMap<QString, QString> fileToContentsMap);
+    FileSearchResultList operator()(QFutureInterface<FileSearchResultList> futureInterface,
+                                          const FileIterator::Item &item) const;
 
 private:
     QMap<QString, QString> fileToContentsMap;
-    QFutureInterface<FileSearchResultList> *future;
     QString searchTermLower;
     QString searchTermUpper;
     int termMaxIndex;
@@ -109,27 +107,25 @@ class FileSearchRegExp
 {
 public:
     FileSearchRegExp(const QString &searchTerm, QTextDocument::FindFlags flags,
-                     QMap<QString, QString> fileToContentsMap,
-                     QFutureInterface<FileSearchResultList> *futureInterface);
-    const FileSearchResultList operator()(const FileIterator::Item &item) const;
+                     QMap<QString, QString> fileToContentsMap);
+    FileSearchRegExp(const FileSearchRegExp &other);
+    FileSearchResultList operator()(QFutureInterface<FileSearchResultList> futureInterface,
+                                          const FileIterator::Item &item) const;
 
 private:
     QRegularExpressionMatch doGuardedMatch(const QString &line, int offset) const;
 
     QMap<QString, QString> fileToContentsMap;
-    QFutureInterface<FileSearchResultList> *future;
     QRegularExpression expression;
     mutable QMutex mutex;
 };
 
 FileSearch::FileSearch(const QString &searchTerm, QTextDocument::FindFlags flags,
-                       QMap<QString, QString> fileToContentsMap,
-                       QFutureInterface<FileSearchResultList> *futureInterface)
+                       QMap<QString, QString> fileToContentsMap)
 {
     this->fileToContentsMap = fileToContentsMap;
     caseSensitive = (flags & QTextDocument::FindCaseSensitively);
     wholeWord = (flags & QTextDocument::FindWholeWords);
-    future = futureInterface;
     searchTermLower = searchTerm.toLower();
     searchTermUpper = searchTerm.toUpper();
     termMaxIndex = searchTerm.length() - 1;
@@ -138,10 +134,11 @@ FileSearch::FileSearch(const QString &searchTerm, QTextDocument::FindFlags flags
     termDataUpper = searchTermUpper.constData();
 }
 
-const FileSearchResultList FileSearch::operator()(const FileIterator::Item &item) const
+FileSearchResultList FileSearch::operator()(QFutureInterface<FileSearchResultList> futureInterface,
+                                                  const FileIterator::Item &item) const
 {
     FileSearchResultList results;
-    if (future->isCanceled())
+    if (futureInterface.isCanceled())
         return results;
     QFile file;
     QTextStream stream;
@@ -212,9 +209,9 @@ const FileSearchResultList FileSearch::operator()(const FileIterator::Item &item
                 }
             }
         }
-        if (future->isPaused())
-            future->waitForResume();
-        if (future->isCanceled())
+        if (futureInterface.isPaused())
+            futureInterface.waitForResume();
+        if (futureInterface.isCanceled())
             break;
     }
     if (file.isOpen())
@@ -223,11 +220,9 @@ const FileSearchResultList FileSearch::operator()(const FileIterator::Item &item
 }
 
 FileSearchRegExp::FileSearchRegExp(const QString &searchTerm, QTextDocument::FindFlags flags,
-                                   QMap<QString, QString> fileToContentsMap,
-                                   QFutureInterface<FileSearchResultList> *futureInterface)
+                                   QMap<QString, QString> fileToContentsMap)
 {
     this->fileToContentsMap = fileToContentsMap;
-    future = futureInterface;
     QString term = searchTerm;
     if (flags & QTextDocument::FindWholeWords)
         term = QString::fromLatin1("\\b%1\\b").arg(term);
@@ -236,16 +231,23 @@ FileSearchRegExp::FileSearchRegExp(const QString &searchTerm, QTextDocument::Fin
     expression = QRegularExpression(term, patternOptions);
 }
 
+FileSearchRegExp::FileSearchRegExp(const FileSearchRegExp &other)
+    : fileToContentsMap(other.fileToContentsMap),
+      expression(other.expression)
+{
+}
+
 QRegularExpressionMatch FileSearchRegExp::doGuardedMatch(const QString &line, int offset) const
 {
     QMutexLocker lock(&mutex);
     return expression.match(line, offset);
 }
 
-const FileSearchResultList FileSearchRegExp::operator()(const FileIterator::Item &item) const
+FileSearchResultList FileSearchRegExp::operator()(QFutureInterface<FileSearchResultList> futureInterface,
+                                                        const FileIterator::Item &item) const
 {
     FileSearchResultList results;
-    if (future->isCanceled())
+    if (futureInterface.isCanceled())
         return results;
     QFile file;
     QTextStream stream;
@@ -273,9 +275,9 @@ const FileSearchResultList FileSearchRegExp::operator()(const FileIterator::Item
             if (pos >= lengthOfLine)
                 break;
         }
-        if (future->isPaused())
-            future->waitForResume();
-        if (future->isCanceled())
+        if (futureInterface.isPaused())
+            futureInterface.waitForResume();
+        if (futureInterface.isCanceled())
             break;
     }
     if (file.isOpen())
@@ -283,140 +285,90 @@ const FileSearchResultList FileSearchRegExp::operator()(const FileIterator::Item
     return results;
 }
 
-class RunFileSearch
+struct SearchState
 {
-public:
-    RunFileSearch(QFutureInterface<FileSearchResultList> &future,
-                  const QString &searchTerm,
-                  FileIterator *files,
-                  const std::function<FileSearchResultList(FileIterator::Item)> &searchFunction);
-
-    void run();
-    void collect(const FileSearchResultList &results);
-
-private:
-    QFutureInterface<FileSearchResultList> &m_future;
-    QString m_searchTerm;
-    FileIterator *m_files;
-    std::function<FileSearchResultList(FileIterator::Item)> m_searchFunction;
-
-    int m_numFilesSearched;
-    int m_numMatches;
-    FileSearchResultList m_results;
-    bool m_canceled;
+    SearchState(const QString &term, FileIterator *iterator) : searchTerm(term), files(iterator) {}
+    QString searchTerm;
+    FileIterator *files = 0;
+    FileSearchResultList cachedResults;
+    int numFilesSearched = 0;
+    int numMatches = 0;
 };
 
-RunFileSearch::RunFileSearch(QFutureInterface<FileSearchResultList> &future,
-                             const QString &searchTerm, FileIterator *files,
-                             const std::function<FileSearchResultList (FileIterator::Item)> &searchFunction)
-    : m_future(future),
-      m_searchTerm(searchTerm),
-      m_files(files),
-      m_searchFunction(searchFunction),
-      m_numFilesSearched(0),
-      m_numMatches(0),
-      m_canceled(false)
+SearchState initFileSearch(QFutureInterface<FileSearchResultList> &futureInterface,
+                           const QString &searchTerm, FileIterator *files)
 {
-    m_future.setProgressRange(0, m_files->maxProgress());
-    m_future.setProgressValueAndText(m_files->currentProgress(), msgFound(m_searchTerm,
-                                                                          m_numMatches,
-                                                                          m_numFilesSearched));
+    futureInterface.setProgressRange(0, files->maxProgress());
+    futureInterface.setProgressValueAndText(files->currentProgress(), msgFound(searchTerm, 0, 0));
+    return SearchState(searchTerm, files);
 }
 
-void RunFileSearch::run()
+void collectSearchResults(QFutureInterface<FileSearchResultList> &futureInterface,
+                          SearchState &state,
+                          const FileSearchResultList &results)
 {
-    // This thread waits for blockingMappedReduced to finish, so reduce the pool's used thread count
-    // so the blockingMappedReduced can use one more thread, and increase it again afterwards.
-    QThreadPool::globalInstance()->releaseThread();
-    QtConcurrent::blockingMappedReduced<FileSearchResultList>(m_files->begin(), m_files->end(),
-                                                  m_searchFunction,
-                                                  [this](FileSearchResultList &, const FileSearchResultList &results) {
-                                                      collect(results);
-                                                  },
-                                                  QtConcurrent::OrderedReduce | QtConcurrent::SequentialReduce);
-    QThreadPool::globalInstance()->reserveThread();
-    if (!m_results.isEmpty()) {
-        m_future.reportResult(m_results);
-        m_results.clear();
+    state.numMatches += results.size();
+    state.cachedResults << results;
+    state.numFilesSearched += 1;
+    if (futureInterface.isProgressUpdateNeeded()
+            || futureInterface.progressValue() == 0 /*workaround for regression in Qt*/) {
+        if (!state.cachedResults.isEmpty()) {
+            futureInterface.reportResult(state.cachedResults);
+            state.cachedResults.clear();
         }
-    if (!m_future.isCanceled())
-        m_future.setProgressValueAndText(m_files->currentProgress(), msgFound(m_searchTerm,
-                                                                              m_numMatches,
-                                                                              m_numFilesSearched));
-    delete m_files;
-    if (m_future.isPaused())
-        m_future.waitForResume();
-}
-
-void RunFileSearch::collect(const FileSearchResultList &results)
-{
-    if (m_future.isCanceled()) {
-        if (!m_canceled) {
-            m_future.setProgressValueAndText(m_files->currentProgress(),
-                                             msgCanceled(m_searchTerm,
-                                                         m_numMatches,
-                                                         m_numFilesSearched));
-            m_canceled = true;
-        }
-        return;
-    }
-    m_numMatches += results.size();
-    m_results << results;
-    ++m_numFilesSearched;
-    if (m_future.isProgressUpdateNeeded()
-            || m_future.progressValue() == 0 /*workaround for regression in Qt*/) {
-        if (!m_results.isEmpty()) {
-            m_future.reportResult(m_results);
-            m_results.clear();
-        }
-        m_future.setProgressRange(0, m_files->maxProgress());
-        m_future.setProgressValueAndText(m_files->currentProgress(), msgFound(m_searchTerm,
-                                                                              m_numMatches,
-                                                                              m_numFilesSearched));
+        futureInterface.setProgressRange(0, state.files->maxProgress());
+        futureInterface.setProgressValueAndText(state.files->currentProgress(),
+                                                 msgFound(state.searchTerm,
+                                                          state.numMatches,
+                                                          state.numFilesSearched));
     }
 }
 
-void runFileSearch(QFutureInterface<FileSearchResultList> &future,
-                   QString searchTerm,
-                   FileIterator *files,
-                   QTextDocument::FindFlags flags,
-                   QMap<QString, QString> fileToContentsMap)
+void cleanUpFileSearch(QFutureInterface<FileSearchResultList> &futureInterface,
+                       SearchState &state)
 {
-    FileSearch searchFunction(searchTerm, flags, fileToContentsMap, &future);
-    RunFileSearch search(future, searchTerm, files, std::bind(&FileSearch::operator(),
-                                                              &searchFunction,
-                                                              std::placeholders::_1));
-    search.run();
-}
-
-void runFileSearchRegExp(QFutureInterface<FileSearchResultList> &future,
-                   QString searchTerm,
-                   FileIterator *files,
-                   QTextDocument::FindFlags flags,
-                   QMap<QString, QString> fileToContentsMap)
-{
-    FileSearchRegExp searchFunction(searchTerm, flags, fileToContentsMap, &future);
-    RunFileSearch search(future, searchTerm, files, std::bind(&FileSearchRegExp::operator(),
-                                                              &searchFunction,
-                                                              std::placeholders::_1));
-    search.run();
+    if (!state.cachedResults.isEmpty()) {
+        futureInterface.reportResult(state.cachedResults);
+        state.cachedResults.clear();
+    }
+    if (futureInterface.isCanceled()) {
+        futureInterface.setProgressValueAndText(state.files->currentProgress(),
+                                                msgCanceled(state.searchTerm,
+                                                            state.numMatches,
+                                                            state.numFilesSearched));
+    } else {
+        futureInterface.setProgressValueAndText(state.files->currentProgress(),
+                                                msgFound(state.searchTerm,
+                                                         state.numMatches,
+                                                         state.numFilesSearched));
+    }
+    delete state.files;
 }
 
 } // namespace
 
-
 QFuture<FileSearchResultList> Utils::findInFiles(const QString &searchTerm, FileIterator *files,
     QTextDocument::FindFlags flags, QMap<QString, QString> fileToContentsMap)
 {
-    return QtConcurrent::run<FileSearchResultList, QString, FileIterator *, QTextDocument::FindFlags, QMap<QString, QString> >
-            (runFileSearch, searchTerm, files, flags, fileToContentsMap);
+    return mapReduce<FileSearchResultList>(std::cref(*files),
+                     [searchTerm, files](QFutureInterface<FileSearchResultList> &futureInterface) {
+                         return initFileSearch(futureInterface, searchTerm, files);
+                     },
+                     FileSearch(searchTerm, flags, fileToContentsMap),
+                     &collectSearchResults,
+                     &cleanUpFileSearch);
 }
 
 QFuture<FileSearchResultList> Utils::findInFilesRegExp(const QString &searchTerm, FileIterator *files,
     QTextDocument::FindFlags flags, QMap<QString, QString> fileToContentsMap)
 {
-    return QtConcurrent::run<FileSearchResultList, QString, FileIterator *, QTextDocument::FindFlags, QMap<QString, QString> >
-            (runFileSearchRegExp, searchTerm, files, flags, fileToContentsMap);
+    return mapReduce<FileSearchResultList>(std::cref(*files),
+                     [searchTerm, files](QFutureInterface<FileSearchResultList> &futureInterface) {
+                         return initFileSearch(futureInterface, searchTerm, files);
+                     },
+                     FileSearchRegExp(searchTerm, flags, fileToContentsMap),
+                     &collectSearchResults,
+                     &cleanUpFileSearch);
 }
 
 QString Utils::expandRegExpReplacement(const QString &replaceText, const QStringList &capturedTexts)
@@ -527,12 +479,12 @@ QString matchCaseReplacement(const QString &originalText, const QString &replace
 
 // #pragma mark -- FileIterator
 
-void FileIterator::next(FileIterator::const_iterator *it)
+void FileIterator::advance(FileIterator::const_iterator *it) const
 {
     if (it->m_index < 0) // == end
         return;
     ++it->m_index;
-    update(it->m_index);
+    const_cast<FileIterator *>(this)->update(it->m_index);
     if (it->m_index < currentFileCount()) {
         it->m_item.filePath = fileAt(it->m_index);
         it->m_item.encoding = codecAt(it->m_index);
@@ -543,9 +495,9 @@ void FileIterator::next(FileIterator::const_iterator *it)
     }
 }
 
-FileIterator::const_iterator FileIterator::begin()
+FileIterator::const_iterator FileIterator::begin() const
 {
-    update(0);
+    const_cast<FileIterator *>(this)->update(0);
     if (currentFileCount() == 0)
         return end();
     return FileIterator::const_iterator(this,
@@ -553,7 +505,7 @@ FileIterator::const_iterator FileIterator::begin()
                                         0/*index*/);
 }
 
-FileIterator::const_iterator FileIterator::end()
+FileIterator::const_iterator FileIterator::end() const
 {
     return FileIterator::const_iterator(this, FileIterator::Item(QString(), 0),
                                         -1/*end*/);

src/libs/utils/filesearch.h

@@ -56,6 +56,8 @@ public:
         QTextCodec *encoding;
     };
 
+    typedef Item value_type;
+
     class const_iterator
     {
     public:
@@ -65,27 +67,27 @@ public:
         typedef const value_type *pointer;
         typedef const value_type &reference;
 
-        const_iterator(FileIterator *parent, Item item, int id)
+        const_iterator(const FileIterator *parent, Item item, int id)
             : m_parent(parent), m_item(item), m_index(id)
         {}
         const Item operator*() const { return m_item; }
         const Item *operator->() const { return &m_item; }
-        void operator++() { m_parent->next(this); }
+        void operator++() { m_parent->advance(this); }
         bool operator==(const const_iterator &other) const
         {
             return m_parent == other.m_parent && m_index == other.m_index;
         }
         bool operator!=(const const_iterator &other) const { return !operator==(other); }
 
-        FileIterator *m_parent;
+        const FileIterator *m_parent;
         Item m_item;
         int m_index; // -1 == end
     };
 
     virtual ~FileIterator() {}
-    void next(const_iterator *it);
-    const_iterator begin();
-    const_iterator end();
+    void advance(const_iterator *it) const;
+    const_iterator begin() const;
+    const_iterator end() const;
 
     virtual int maxProgress() const = 0;
     virtual int currentProgress() const = 0;

src/libs/utils/runextensions.h

@@ -31,12 +31,18 @@
 #ifndef RUNEXTENSIONS_H
 #define RUNEXTENSIONS_H
 
+#include "qtcassert.h"
+
 #include <qrunnable.h>
 #include <qfuture.h>
 #include <qfutureinterface.h>
 #include <qthreadpool.h>
 
+#include <chrono>
 #include <functional>
+#include <future>
+#include <thread>
+#include <vector>
 
 QT_BEGIN_NAMESPACE
 
@@ -431,4 +437,116 @@ QFuture<T> run(const std::function<void (QFutureInterface<T> &)> &fn)
 
 QT_END_NAMESPACE
 
+namespace Utils {
+
+template<typename T>
+typename std::vector<std::future<T>>::iterator
+waitForAny(std::vector<std::future<T>> &futures)
+{
+    // Wait for any future to have a result ready.
+    // Unfortunately we have to do that in a busy loop because future doesn't have a feature to
+    // wait for any of a set of futures (yet? possibly when_any in C++17).
+    auto end = futures.end();
+    QTC_ASSERT(!futures.empty(), return end);
+    auto futureIterator = futures.begin();
+    forever {
+        if (futureIterator->wait_for(std::chrono::duration<quint64>::zero()) == std::future_status::ready)
+            return futureIterator;
+        ++futureIterator;
+        if (futureIterator == end)
+            futureIterator = futures.begin();
+    }
+}
+
+namespace Internal {
+
+template<typename T>
+void swapErase(std::vector<T> &vec, typename std::vector<T>::iterator it)
+{
+    // efficient erasing by swapping with back element
+    *it = std::move(vec.back());
+    vec.pop_back();
+}
+
+template <typename MapResult, typename State, typename ReduceResult, typename ReduceFunction>
+void reduceOne(QFutureInterface<ReduceResult> &futureInterface,
+               std::vector<std::future<MapResult>> &futures,
+               State &state, const ReduceFunction &reduce)
+{
+    auto futureIterator = waitForAny(futures);
+    if (futureIterator != futures.end()) {
+        reduce(futureInterface, state, futureIterator->get());
+        swapErase(futures, futureIterator);
+    }
+}
+
+// This together with reduceOne can be replaced by std::transformReduce (parallelism TS)
+// when that becomes widely available in C++ implementations
+template <typename Container, typename MapFunction, typename State, typename ReduceResult, typename ReduceFunction>
+void mapReduceLoop(QFutureInterface<ReduceResult> &futureInterface, const Container &container,
+               const MapFunction &map, State &state, const ReduceFunction &reduce)
+{
+    const unsigned MAX_THREADS = std::thread::hardware_concurrency();
+    using MapResult = typename std::result_of<MapFunction(QFutureInterface<ReduceResult>,typename Container::value_type)>::type;
+    std::vector<std::future<MapResult>> futures;
+    futures.reserve(MAX_THREADS);
+    auto fileIterator = container.begin();
+    auto end = container.end();
+    while (!futureInterface.isCanceled() && (fileIterator != end || futures.size() != 0)) {
+        if (futures.size() >= MAX_THREADS || fileIterator == end) {
+            // We don't want to start a new thread (yet), so try to find a future that is ready and
+            // handle its result.
+            reduceOne(futureInterface, futures, state, reduce);
+        } else { // start a new thread
+            futures.push_back(std::async(std::launch::async,
+                                         map, futureInterface, *fileIterator));
+            ++fileIterator;
+        }
+    }
+}
+
+template <typename Container, typename InitFunction, typename MapFunction, typename ReduceResult,
+          typename ReduceFunction, typename CleanUpFunction>
+void blockingMapReduce(QFutureInterface<ReduceResult> futureInterface, const Container &container,
+                       const InitFunction &init, const MapFunction &map,
+                       const ReduceFunction &reduce, const CleanUpFunction &cleanup)
+{
+    auto state = init(futureInterface);
+    futureInterface.reportStarted();
+    mapReduceLoop(futureInterface, container, map, state, reduce);
+    cleanup(futureInterface, state);
+    if (futureInterface.isPaused())
+        futureInterface.waitForResume();
+    futureInterface.reportFinished();
+}
+
+} // Internal
+
+template <typename ReduceResult, typename Container, typename InitFunction, typename MapFunction,
+          typename ReduceFunction, typename CleanUpFunction>
+QFuture<ReduceResult> mapReduce(std::reference_wrapper<Container> containerWrapper,
+                                const InitFunction &init, const MapFunction &map,
+                                const ReduceFunction &reduce, const CleanUpFunction &cleanup)
+{
+    auto fi = QFutureInterface<ReduceResult>();
+    QFuture<ReduceResult> future = fi.future();
+    std::thread(Internal::blockingMapReduce<Container, InitFunction, MapFunction, ReduceResult, ReduceFunction, CleanUpFunction>,
+                fi, containerWrapper, init, map, reduce, cleanup).detach();
+    return future;
+}
+
+template <typename ReduceResult, typename Container, typename InitFunction, typename MapFunction,
+          typename ReduceFunction, typename CleanUpFunction>
+QFuture<ReduceResult> mapReduce(const Container &container, const InitFunction &init, const MapFunction &map,
+               const ReduceFunction &reduce, const CleanUpFunction &cleanup)
+{
+    auto fi = QFutureInterface<ReduceResult>();
+    QFuture<ReduceResult> future = fi.future();
+    std::thread(Internal::blockingMapReduce<Container, InitFunction, MapFunction, ReduceResult, ReduceFunction, CleanUpFunction>,
+                fi, container, init, map, reduce, cleanup).detach();
+    return future;
+}
+
+} // Utils
+
 #endif // RUNEXTENSIONS_H