AutoTest: Further optimize TestCodeParser::scanForTests()

When loading a Qt project, after the Scanning For Tests
finished, the scanForTests() blocks the main thread for
about 3.5 seconds on the calls to parser->init().

Refactor the code so that it operates on QSet<FilePath>
instead of QList<FilePaths>.

This patch constraints the freeze to about 40 ms.

Change-Id: I219b3e2abf2b7e5166eec08d83f4cdcb8e4a8098
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>
Reviewed-by: Christian Stenger <christian.stenger@qt.io>
This commit is contained in:
Jarek Kobus
2023-04-23 17:25:46 +02:00
parent 72bddf9f51
commit b9ca680b03
12 changed files with 47 additions and 46 deletions

View File

@@ -24,7 +24,7 @@ CppParser::CppParser(ITestFramework *framework)
{ {
} }
void CppParser::init(const FilePaths &filesToParse, bool fullParse) void CppParser::init(const QSet<FilePath> &filesToParse, bool fullParse)
{ {
Q_UNUSED(filesToParse) Q_UNUSED(filesToParse)
Q_UNUSED(fullParse) Q_UNUSED(fullParse)

View File

@@ -45,7 +45,7 @@ class ITestParser
public: public:
explicit ITestParser(ITestFramework *framework) : m_framework(framework) {} explicit ITestParser(ITestFramework *framework) : m_framework(framework) {}
virtual ~ITestParser() { } virtual ~ITestParser() { }
virtual void init(const Utils::FilePaths &filesToParse, bool fullParse) = 0; virtual void init(const QSet<Utils::FilePath> &filesToParse, bool fullParse) = 0;
virtual bool processDocument(QPromise<TestParseResultPtr> &futureInterface, virtual bool processDocument(QPromise<TestParseResultPtr> &futureInterface,
const Utils::FilePath &fileName) = 0; const Utils::FilePath &fileName) = 0;
@@ -63,7 +63,7 @@ class CppParser : public ITestParser
{ {
public: public:
explicit CppParser(ITestFramework *framework); explicit CppParser(ITestFramework *framework);
void init(const Utils::FilePaths &filesToParse, bool fullParse) override; void init(const QSet<Utils::FilePath> &filesToParse, bool fullParse) override;
static bool selectedForBuilding(const Utils::FilePath &fileName); static bool selectedForBuilding(const Utils::FilePath &fileName);
QByteArray getFileContent(const Utils::FilePath &filePath) const; QByteArray getFileContent(const Utils::FilePath &filePath) const;
void release() override; void release() override;

View File

@@ -4,7 +4,6 @@
#include "qttest_utils.h" #include "qttest_utils.h"
#include "qttesttreeitem.h" #include "qttesttreeitem.h"
#include "../autotestplugin.h" #include "../autotestplugin.h"
#include "../testframeworkmanager.h"
#include "../testsettings.h" #include "../testsettings.h"
#include <utils/algorithm.h> #include <utils/algorithm.h>
@@ -27,7 +26,8 @@ bool isQTestMacro(const QByteArray &macro)
return valid.contains(macro); return valid.contains(macro);
} }
QHash<FilePath, TestCases> testCaseNamesForFiles(ITestFramework *framework, const FilePaths &files) QHash<FilePath, TestCases> testCaseNamesForFiles(ITestFramework *framework,
const QSet<FilePath> &files)
{ {
QHash<FilePath, TestCases> result; QHash<FilePath, TestCases> result;
TestTreeItem *rootNode = framework->rootNode(); TestTreeItem *rootNode = framework->rootNode();
@@ -49,7 +49,8 @@ QHash<FilePath, TestCases> testCaseNamesForFiles(ITestFramework *framework, cons
return result; return result;
} }
QMultiHash<FilePath, FilePath> alternativeFiles(ITestFramework *framework, const FilePaths &files) QMultiHash<FilePath, FilePath> alternativeFiles(ITestFramework *framework,
const QSet<FilePath> &files)
{ {
QMultiHash<FilePath, FilePath> result; QMultiHash<FilePath, FilePath> result;
TestTreeItem *rootNode = framework->rootNode(); TestTreeItem *rootNode = framework->rootNode();

View File

@@ -26,9 +26,9 @@ namespace QTestUtils {
bool isQTestMacro(const QByteArray &macro); bool isQTestMacro(const QByteArray &macro);
QHash<Utils::FilePath, TestCases> testCaseNamesForFiles(ITestFramework *framework, QHash<Utils::FilePath, TestCases> testCaseNamesForFiles(ITestFramework *framework,
const Utils::FilePaths &files); const QSet<Utils::FilePath> &files);
QMultiHash<Utils::FilePath, Utils::FilePath> alternativeFiles(ITestFramework *framework, QMultiHash<Utils::FilePath, Utils::FilePath> alternativeFiles(ITestFramework *framework,
const Utils::FilePaths &files); const QSet<Utils::FilePath> &files);
QStringList filterInterfering(const QStringList &provided, QStringList *omitted, bool isQuickTest); QStringList filterInterfering(const QStringList &provided, QStringList *omitted, bool isQuickTest);
Utils::Environment prepareBasicEnvironment(const Utils::Environment &env); Utils::Environment prepareBasicEnvironment(const Utils::Environment &env);

View File

@@ -414,7 +414,7 @@ QtTestParseResult *QtTestParser::createParseResult(
return parseResult; return parseResult;
} }
void QtTestParser::init(const FilePaths &filesToParse, bool fullParse) void QtTestParser::init(const QSet<FilePath> &filesToParse, bool fullParse)
{ {
if (!fullParse) { // in a full parse cached information might lead to wrong results if (!fullParse) { // in a full parse cached information might lead to wrong results
m_testCases = QTestUtils::testCaseNamesForFiles(framework(), filesToParse); m_testCases = QTestUtils::testCaseNamesForFiles(framework(), filesToParse);

View File

@@ -34,7 +34,7 @@ class QtTestParser : public CppParser
public: public:
explicit QtTestParser(ITestFramework *framework) : CppParser(framework) {} explicit QtTestParser(ITestFramework *framework) : CppParser(framework) {}
void init(const Utils::FilePaths &filesToParse, bool fullParse) override; void init(const QSet<Utils::FilePath> &filesToParse, bool fullParse) override;
void release() override; void release() override;
bool processDocument(QPromise<TestParseResultPtr> &promise, bool processDocument(QPromise<TestParseResultPtr> &promise,
const Utils::FilePath &fileName) override; const Utils::FilePath &fileName) override;

View File

@@ -22,7 +22,8 @@ bool isQuickTestMacro(const QByteArray &macro)
return valid.contains(macro); return valid.contains(macro);
} }
QHash<FilePath, FilePath> proFilesForQmlFiles(ITestFramework *framework, const FilePaths &files) QHash<FilePath, FilePath> proFilesForQmlFiles(ITestFramework *framework,
const QSet<FilePath> &files)
{ {
QHash<FilePath, FilePath> result; QHash<FilePath, FilePath> result;
TestTreeItem *rootNode = framework->rootNode(); TestTreeItem *rootNode = framework->rootNode();

View File

@@ -16,7 +16,7 @@ namespace QuickTestUtils {
bool isQuickTestMacro(const QByteArray &macro); bool isQuickTestMacro(const QByteArray &macro);
QHash<Utils::FilePath, Utils::FilePath> proFilesForQmlFiles(ITestFramework *framework, QHash<Utils::FilePath, Utils::FilePath> proFilesForQmlFiles(ITestFramework *framework,
const Utils::FilePaths &files); const QSet<Utils::FilePath> &files);
} // namespace QuickTestUtils } // namespace QuickTestUtils
} // namespace Internal } // namespace Internal

View File

@@ -338,7 +338,7 @@ QuickTestParser::QuickTestParser(ITestFramework *framework)
this, &QuickTestParser::handleDirectoryChanged); this, &QuickTestParser::handleDirectoryChanged);
} }
void QuickTestParser::init(const FilePaths &filesToParse, bool fullParse) void QuickTestParser::init(const QSet<FilePath> &filesToParse, bool fullParse)
{ {
m_qmlSnapshot = QmlJSTools::Internal::ModelManager::instance()->snapshot(); m_qmlSnapshot = QmlJSTools::Internal::ModelManager::instance()->snapshot();
if (!fullParse) { if (!fullParse) {

View File

@@ -24,7 +24,7 @@ class QuickTestParser : public QObject, public CppParser
Q_OBJECT Q_OBJECT
public: public:
explicit QuickTestParser(ITestFramework *framework); explicit QuickTestParser(ITestFramework *framework);
void init(const Utils::FilePaths &filesToParse, bool fullParse) override; void init(const QSet<Utils::FilePath> &filesToParse, bool fullParse) override;
void release() override; void release() override;
bool processDocument(QPromise<TestParseResultPtr> &promise, bool processDocument(QPromise<TestParseResultPtr> &promise,
const Utils::FilePath &fileName) override; const Utils::FilePath &fileName) override;

View File

@@ -87,7 +87,7 @@ void TestCodeParser::setState(State state)
m_postponedUpdateType = UpdateType::NoUpdate; m_postponedUpdateType = UpdateType::NoUpdate;
qCDebug(LOG) << "calling scanForTests with postponed files (setState)"; qCDebug(LOG) << "calling scanForTests with postponed files (setState)";
if (!m_reparseTimer.isActive()) if (!m_reparseTimer.isActive())
scanForTests(Utils::toList(m_postponedFiles)); scanForTests(m_postponedFiles);
} }
} }
} }
@@ -213,27 +213,28 @@ void TestCodeParser::aboutToShutdown()
m_futureSynchronizer.waitForFinished(); m_futureSynchronizer.waitForFinished();
} }
bool TestCodeParser::postponed(const FilePaths &fileList) bool TestCodeParser::postponed(const QSet<FilePath> &filePaths)
{ {
switch (m_parserState) { switch (m_parserState) {
case Idle: case Idle:
if (fileList.size() == 1) { if (filePaths.size() == 1) {
if (m_reparseTimerTimedOut) if (m_reparseTimerTimedOut)
return false; return false;
const FilePath filePath = *filePaths.begin();
switch (m_postponedFiles.size()) { switch (m_postponedFiles.size()) {
case 0: case 0:
m_postponedFiles.insert(fileList.first()); m_postponedFiles.insert(filePath);
m_reparseTimer.setInterval(1000); m_reparseTimer.setInterval(1000);
m_reparseTimer.start(); m_reparseTimer.start();
return true; return true;
case 1: case 1:
if (m_postponedFiles.contains(fileList.first())) { if (m_postponedFiles.contains(filePath)) {
m_reparseTimer.start(); m_reparseTimer.start();
return true; return true;
} }
Q_FALLTHROUGH(); Q_FALLTHROUGH();
default: default:
m_postponedFiles.insert(fileList.first()); m_postponedFiles.insert(filePath);
m_reparseTimer.stop(); m_reparseTimer.stop();
m_reparseTimer.setInterval(0); m_reparseTimer.setInterval(0);
m_reparseTimerTimedOut = false; m_reparseTimerTimedOut = false;
@@ -245,7 +246,7 @@ bool TestCodeParser::postponed(const FilePaths &fileList)
case PartialParse: case PartialParse:
case FullParse: case FullParse:
// parse is running, postponing a full parse // parse is running, postponing a full parse
if (fileList.isEmpty()) { if (filePaths.isEmpty()) {
m_postponedFiles.clear(); m_postponedFiles.clear();
m_postponedUpdateType = UpdateType::FullUpdate; m_postponedUpdateType = UpdateType::FullUpdate;
qCDebug(LOG) << "Canceling scanForTest (full parse triggered while running a scan)"; qCDebug(LOG) << "Canceling scanForTest (full parse triggered while running a scan)";
@@ -255,8 +256,7 @@ bool TestCodeParser::postponed(const FilePaths &fileList)
if (m_postponedUpdateType == UpdateType::FullUpdate) if (m_postponedUpdateType == UpdateType::FullUpdate)
return true; return true;
// partial parse triggered, postpone or add current files to already postponed partial // partial parse triggered, postpone or add current files to already postponed partial
for (const FilePath &file : fileList) m_postponedFiles += filePaths;
m_postponedFiles.insert(file);
m_postponedUpdateType = UpdateType::PartialUpdate; m_postponedUpdateType = UpdateType::PartialUpdate;
} }
return true; return true;
@@ -277,31 +277,32 @@ static void parseFileForTests(QPromise<TestParseResultPtr> &promise,
} }
} }
void TestCodeParser::scanForTests(const FilePaths &fileList, const QList<ITestParser *> &parsers) void TestCodeParser::scanForTests(const QSet<FilePath> &filePaths,
const QList<ITestParser *> &parsers)
{ {
if (m_parserState == Shutdown || m_testCodeParsers.isEmpty()) if (m_parserState == Shutdown || m_testCodeParsers.isEmpty())
return; return;
if (postponed(fileList)) if (postponed(filePaths))
return; return;
QSet<FilePath> files = filePaths; // avoid getting cleared if m_postponedFiles have been passed
m_reparseTimer.stop(); m_reparseTimer.stop();
m_reparseTimerTimedOut = false; m_reparseTimerTimedOut = false;
m_postponedFiles.clear(); m_postponedFiles.clear();
bool isFullParse = fileList.isEmpty(); const bool isFullParse = files.isEmpty();
Project *project = ProjectManager::startupProject(); Project *project = ProjectManager::startupProject();
if (!project) if (!project)
return; return;
FilePaths list;
if (isFullParse) { if (isFullParse) {
list = project->files(Project::SourceFiles); files = Utils::toSet(project->files(Project::SourceFiles));
if (list.isEmpty()) { if (files.isEmpty()) {
// at least project file should be there, but might happen if parsing current project // at least project file should be there, but might happen if parsing current project
// takes too long, especially when opening sessions holding multiple projects // takes too long, especially when opening sessions holding multiple projects
qCDebug(LOG) << "File list empty (FullParse) - trying again in a sec"; qCDebug(LOG) << "File list empty (FullParse) - trying again in a sec";
emitUpdateTestTree(); emitUpdateTestTree();
return; return;
} else if (list.size() == 1 && list.first() == project->projectFilePath()) { } else if (files.size() == 1 && *files.constBegin() == project->projectFilePath()) {
qCDebug(LOG) << "File list contains only the project file."; qCDebug(LOG) << "File list contains only the project file.";
return; return;
} }
@@ -309,7 +310,6 @@ void TestCodeParser::scanForTests(const FilePaths &fileList, const QList<ITestPa
qCDebug(LOG) << "setting state to FullParse (scanForTests)"; qCDebug(LOG) << "setting state to FullParse (scanForTests)";
m_parserState = FullParse; m_parserState = FullParse;
} else { } else {
list << fileList;
qCDebug(LOG) << "setting state to PartialParse (scanForTests)"; qCDebug(LOG) << "setting state to PartialParse (scanForTests)";
m_parserState = PartialParse; m_parserState = PartialParse;
} }
@@ -318,7 +318,7 @@ void TestCodeParser::scanForTests(const FilePaths &fileList, const QList<ITestPa
TestTreeModel::instance()->updateCheckStateCache(); TestTreeModel::instance()->updateCheckStateCache();
if (isFullParse) { if (isFullParse) {
// remove qml files as they will be found automatically by the referencing cpp file // remove qml files as they will be found automatically by the referencing cpp file
list = Utils::filtered(list, [](const FilePath &fn) { return !fn.endsWith(".qml"); }); files = Utils::filtered(files, [](const FilePath &fn) { return !fn.endsWith(".qml"); });
if (!parsers.isEmpty()) { if (!parsers.isEmpty()) {
for (ITestParser *parser : parsers) for (ITestParser *parser : parsers)
parser->framework()->rootNode()->markForRemovalRecursively(true); parser->framework()->rootNode()->markForRemovalRecursively(true);
@@ -326,15 +326,14 @@ void TestCodeParser::scanForTests(const FilePaths &fileList, const QList<ITestPa
emit requestRemoveAllFrameworkItems(); emit requestRemoveAllFrameworkItems();
} }
} else if (!parsers.isEmpty()) { } else if (!parsers.isEmpty()) {
const auto set = Utils::toSet(list);
for (ITestParser *parser: parsers) { for (ITestParser *parser: parsers) {
parser->framework()->rootNode()->markForRemovalRecursively(set); parser->framework()->rootNode()->markForRemovalRecursively(files);
} }
} else { } else {
emit requestRemoval(Utils::toSet(list)); emit requestRemoval(files);
} }
QTC_ASSERT(!(isFullParse && list.isEmpty()), onFinished(true); return); QTC_ASSERT(!(isFullParse && files.isEmpty()), onFinished(true); return);
// use only a single parser or all current active? // use only a single parser or all current active?
const QList<ITestParser *> codeParsers = parsers.isEmpty() ? m_testCodeParsers : parsers; const QList<ITestParser *> codeParsers = parsers.isEmpty() ? m_testCodeParsers : parsers;
@@ -343,14 +342,14 @@ void TestCodeParser::scanForTests(const FilePaths &fileList, const QList<ITestPa
const auto cppSnapshot = CppEditor::CppModelManager::instance()->snapshot(); const auto cppSnapshot = CppEditor::CppModelManager::instance()->snapshot();
for (ITestParser *parser : codeParsers) { for (ITestParser *parser : codeParsers) {
parser->init(list, isFullParse); parser->init(files, isFullParse);
for (const QString &ext : parser->supportedExtensions()) for (const QString &ext : parser->supportedExtensions())
extensions.insert(ext); extensions.insert(ext);
} }
// We are only interested in files that have been either parsed by the c++ parser, // We are only interested in files that have been either parsed by the c++ parser,
// or have an extension that one of the parsers is specifically interested in. // or have an extension that one of the parsers is specifically interested in.
const FilePaths filteredList const QSet<FilePath> filteredFiles
= Utils::filtered(list, [&extensions, &cppSnapshot](const FilePath &fn) { = Utils::filtered(files, [&extensions, &cppSnapshot](const FilePath &fn) {
const bool isSupportedExtension = Utils::anyOf(extensions, [&fn](const QString &ext) { const bool isSupportedExtension = Utils::anyOf(extensions, [&fn](const QString &ext) {
return fn.suffix() == ext; return fn.suffix() == ext;
}); });
@@ -359,13 +358,13 @@ void TestCodeParser::scanForTests(const FilePaths &fileList, const QList<ITestPa
return cppSnapshot.contains(fn); return cppSnapshot.contains(fn);
}); });
qCDebug(LOG) << "Starting scan of" << filteredList.size() << "(" << list.size() << ")" qCDebug(LOG) << "Starting scan of" << filteredFiles.size() << "(" << files.size() << ")"
<< "files with" << codeParsers.size() << "parsers"; << "files with" << codeParsers.size() << "parsers";
using namespace Tasking; using namespace Tasking;
QList<TaskItem> tasks{parallel}; // TODO: use ParallelLimit(N) and add to settings? QList<TaskItem> tasks{parallel}; // TODO: use ParallelLimit(N) and add to settings?
for (const FilePath &file : filteredList) { for (const FilePath &file : filteredFiles) {
const auto setup = [this, codeParsers, file](AsyncTask<TestParseResultPtr> &async) { const auto setup = [this, codeParsers, file](AsyncTask<TestParseResultPtr> &async) {
async.setConcurrentCallData(parseFileForTests, codeParsers, file); async.setConcurrentCallData(parseFileForTests, codeParsers, file);
async.setThreadPool(m_threadPool); async.setThreadPool(m_threadPool);
@@ -384,7 +383,7 @@ void TestCodeParser::scanForTests(const FilePaths &fileList, const QList<ITestPa
connect(m_taskTree.get(), &TaskTree::started, this, &TestCodeParser::parsingStarted); connect(m_taskTree.get(), &TaskTree::started, this, &TestCodeParser::parsingStarted);
connect(m_taskTree.get(), &TaskTree::done, this, onDone); connect(m_taskTree.get(), &TaskTree::done, this, onDone);
connect(m_taskTree.get(), &TaskTree::errorOccurred, this, onError); connect(m_taskTree.get(), &TaskTree::errorOccurred, this, onError);
if (filteredList.size() > 5) { if (filteredFiles.size() > 5) {
auto progress = new TaskProgress(m_taskTree.get()); auto progress = new TaskProgress(m_taskTree.get());
progress->setDisplayName(Tr::tr("Scanning for Tests")); progress->setDisplayName(Tr::tr("Scanning for Tests"));
progress->setId(Constants::TASK_PARSE); progress->setId(Constants::TASK_PARSE);
@@ -467,7 +466,7 @@ void TestCodeParser::onPartialParsingFinished()
case UpdateType::PartialUpdate: case UpdateType::PartialUpdate:
qCDebug(LOG) << "calling scanForTests with postponed files (onPartialParsingFinished)"; qCDebug(LOG) << "calling scanForTests with postponed files (onPartialParsingFinished)";
if (!m_reparseTimer.isActive()) if (!m_reparseTimer.isActive())
scanForTests(Utils::toList(m_postponedFiles)); scanForTests(m_postponedFiles);
break; break;
case UpdateType::NoUpdate: case UpdateType::NoUpdate:
m_dirty |= m_codeModelParsing; m_dirty |= m_codeModelParsing;
@@ -491,7 +490,7 @@ void TestCodeParser::onPartialParsingFinished()
void TestCodeParser::parsePostponedFiles() void TestCodeParser::parsePostponedFiles()
{ {
m_reparseTimerTimedOut = true; m_reparseTimerTimedOut = true;
scanForTests(Utils::toList(m_postponedFiles)); scanForTests(m_postponedFiles);
} }
void TestCodeParser::releaseParserInternals() void TestCodeParser::releaseParserInternals()

View File

@@ -66,8 +66,8 @@ public:
void aboutToShutdown(); void aboutToShutdown();
private: private:
bool postponed(const Utils::FilePaths &fileList); bool postponed(const QSet<Utils::FilePath> &fileList);
void scanForTests(const Utils::FilePaths &fileList = Utils::FilePaths(), void scanForTests(const QSet<Utils::FilePath> &filePaths = {},
const QList<ITestParser *> &parsers = {}); const QList<ITestParser *> &parsers = {});
// qml files must be handled slightly different // qml files must be handled slightly different