From 0e012a835b4d7e086eb954a210a10871dc28726e Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Wed, 16 Dec 2015 14:58:04 +0100 Subject: [PATCH] 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 --- src/libs/utils/filesearch.cpp | 232 +++++++++++++-------------------- src/libs/utils/filesearch.h | 14 +- src/libs/utils/runextensions.h | 118 +++++++++++++++++ 3 files changed, 218 insertions(+), 146 deletions(-) diff --git a/src/libs/utils/filesearch.cpp b/src/libs/utils/filesearch.cpp index 580f43edd7d..2dc09296890 100644 --- a/src/libs/utils/filesearch.cpp +++ b/src/libs/utils/filesearch.cpp @@ -35,7 +35,6 @@ #include #include #include -#include #include @@ -88,13 +87,12 @@ class FileSearch { public: FileSearch(const QString &searchTerm, QTextDocument::FindFlags flags, - QMap fileToContentsMap, - QFutureInterface *futureInterface); - const FileSearchResultList operator()(const FileIterator::Item &item) const; + QMap fileToContentsMap); + FileSearchResultList operator()(QFutureInterface futureInterface, + const FileIterator::Item &item) const; private: QMap fileToContentsMap; - QFutureInterface *future; QString searchTermLower; QString searchTermUpper; int termMaxIndex; @@ -109,27 +107,25 @@ class FileSearchRegExp { public: FileSearchRegExp(const QString &searchTerm, QTextDocument::FindFlags flags, - QMap fileToContentsMap, - QFutureInterface *futureInterface); - const FileSearchResultList operator()(const FileIterator::Item &item) const; + QMap fileToContentsMap); + FileSearchRegExp(const FileSearchRegExp &other); + FileSearchResultList operator()(QFutureInterface futureInterface, + const FileIterator::Item &item) const; private: QRegularExpressionMatch doGuardedMatch(const QString &line, int offset) const; QMap fileToContentsMap; - QFutureInterface *future; QRegularExpression expression; mutable QMutex mutex; }; FileSearch::FileSearch(const QString &searchTerm, QTextDocument::FindFlags flags, - QMap fileToContentsMap, - QFutureInterface *futureInterface) + QMap 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 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 fileToContentsMap, - QFutureInterface *futureInterface) + QMap 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 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 &future, - const QString &searchTerm, - FileIterator *files, - const std::function &searchFunction); - - void run(); - void collect(const FileSearchResultList &results); - -private: - QFutureInterface &m_future; - QString m_searchTerm; - FileIterator *m_files; - std::function 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 &future, - const QString &searchTerm, FileIterator *files, - const std::function &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 &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 &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(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(); - } - 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; + 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(); } - 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 &future, - QString searchTerm, - FileIterator *files, - QTextDocument::FindFlags flags, - QMap fileToContentsMap) +void cleanUpFileSearch(QFutureInterface &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 &future, - QString searchTerm, - FileIterator *files, - QTextDocument::FindFlags flags, - QMap 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 Utils::findInFiles(const QString &searchTerm, FileIterator *files, QTextDocument::FindFlags flags, QMap fileToContentsMap) { - return QtConcurrent::run > - (runFileSearch, searchTerm, files, flags, fileToContentsMap); + return mapReduce(std::cref(*files), + [searchTerm, files](QFutureInterface &futureInterface) { + return initFileSearch(futureInterface, searchTerm, files); + }, + FileSearch(searchTerm, flags, fileToContentsMap), + &collectSearchResults, + &cleanUpFileSearch); } QFuture Utils::findInFilesRegExp(const QString &searchTerm, FileIterator *files, QTextDocument::FindFlags flags, QMap fileToContentsMap) { - return QtConcurrent::run > - (runFileSearchRegExp, searchTerm, files, flags, fileToContentsMap); + return mapReduce(std::cref(*files), + [searchTerm, files](QFutureInterface &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(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(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*/); diff --git a/src/libs/utils/filesearch.h b/src/libs/utils/filesearch.h index bf827d0c274..3c3195fea96 100644 --- a/src/libs/utils/filesearch.h +++ b/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; diff --git a/src/libs/utils/runextensions.h b/src/libs/utils/runextensions.h index 0bf0d94b7b9..9f5d88a6a12 100644 --- a/src/libs/utils/runextensions.h +++ b/src/libs/utils/runextensions.h @@ -31,12 +31,18 @@ #ifndef RUNEXTENSIONS_H #define RUNEXTENSIONS_H +#include "qtcassert.h" + #include #include #include #include +#include #include +#include +#include +#include QT_BEGIN_NAMESPACE @@ -431,4 +437,116 @@ QFuture run(const std::function &)> &fn) QT_END_NAMESPACE +namespace Utils { + +template +typename std::vector>::iterator +waitForAny(std::vector> &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::zero()) == std::future_status::ready) + return futureIterator; + ++futureIterator; + if (futureIterator == end) + futureIterator = futures.begin(); + } +} + +namespace Internal { + +template +void swapErase(std::vector &vec, typename std::vector::iterator it) +{ + // efficient erasing by swapping with back element + *it = std::move(vec.back()); + vec.pop_back(); +} + +template +void reduceOne(QFutureInterface &futureInterface, + std::vector> &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 +void mapReduceLoop(QFutureInterface &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,typename Container::value_type)>::type; + std::vector> 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 +void blockingMapReduce(QFutureInterface 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 +QFuture mapReduce(std::reference_wrapper containerWrapper, + const InitFunction &init, const MapFunction &map, + const ReduceFunction &reduce, const CleanUpFunction &cleanup) +{ + auto fi = QFutureInterface(); + QFuture future = fi.future(); + std::thread(Internal::blockingMapReduce, + fi, containerWrapper, init, map, reduce, cleanup).detach(); + return future; +} + +template +QFuture mapReduce(const Container &container, const InitFunction &init, const MapFunction &map, + const ReduceFunction &reduce, const CleanUpFunction &cleanup) +{ + auto fi = QFutureInterface(); + QFuture future = fi.future(); + std::thread(Internal::blockingMapReduce, + fi, container, init, map, reduce, cleanup).detach(); + return future; +} + +} // Utils + #endif // RUNEXTENSIONS_H