Commit 442bdbde authored by Nikolai Kosjar's avatar Nikolai Kosjar

CppTools: Avoid unnecessary blocking of main thread

Among others, BaseEditorDocumentParser::projectPart() was a blocking
operation in case the parser was running. This led to noticeable GUI
freezes for the ClangCodeModel since the function was called from the
main thread.

Rework *EditorDocumentParser to clearly separate the configuration data
(input) from the actual object state. Querying/setting configuration or
(last) state does not block anymore. update() is supposed to get the
necessary configuration and the last state at the beginning and to set
the new state at end.

Change-Id: Ib4b534fa6ff373c3059826726b3f10ece95acc21
Reviewed-by: default avatarErik Verbruggen <erik.verbruggen@theqtcompany.com>
parent 91c497b1
......@@ -91,21 +91,25 @@ ClangEditorDocumentParser::ClangEditorDocumentParser(const QString &filePath)
void ClangEditorDocumentParser::update(CppTools::WorkingCopy workingCopy)
{
QTC_ASSERT(m_marker, return);
QMutexLocker lock(m_marker->mutex());
QMutexLocker lock2(&m_mutex);
QMutexLocker locker(&m_updateIsRunning);
m_projectPart = determineProjectPart();
const QStringList options = createOptions(filePath(), projectPart(), true);
// Determine project part
State state_ = state();
state_.projectPart = determineProjectPart(filePath(), configuration(), state_);
setState(state_);
// Determine command line arguments
const QStringList options = createOptions(filePath(), state_.projectPart, true);
qCDebug(log, "Reparse options (cmd line equivalent): %s",
commandLine(options, filePath()).toUtf8().constData());
QTime t; t.start();
// Run
QTime t; t.start();
QMutexLocker lock(m_marker->mutex());
m_marker->setFileName(filePath());
m_marker->setCompilationOptions(options);
const Internal::UnsavedFiles unsavedFiles = Utils::createUnsavedFiles(workingCopy);
m_marker->reparse(unsavedFiles);
qCDebug(log) << "Reparse took" << t.elapsed() << "ms.";
}
......
......@@ -268,9 +268,11 @@ void CppEditorDocument::setPreprocessorSettings(const CppTools::ProjectPart::Ptr
{
CppTools::BaseEditorDocumentParser *parser = processor()->parser();
QTC_ASSERT(parser, return);
if (parser->projectPart() != projectPart || parser->editorDefines() != defines) {
parser->setProjectPart(projectPart);
parser->setEditorDefines(defines);
if (parser->projectPart() != projectPart || parser->configuration().editorDefines != defines) {
CppTools::BaseEditorDocumentParser::Configuration config = parser->configuration();
config.manuallySetProjectPart = projectPart;
config.editorDefines = defines;
parser->setConfiguration(config);
emit preprocessorSettingsChanged(!defines.trimmed().isEmpty());
}
......
......@@ -44,15 +44,19 @@ namespace CppTools {
It's meant to be used in the C++ editor to get precise results by using
the "best" project part for a file.
Derived classes are expected to implement update() by using the protected
mutex, determineProjectPart() and by respecting the options set by the client.
Derived classes are expected to implement update() this way:
\list
\li Get a copy of the configuration and the last state.
\li Acquire the protected m_updateIsRunning for the duration of update().
\li Work on the data and do whatever is necessary. At least, update
the project part with the help of determineProjectPart().
\li Ensure the new state is set before update() returns.
\endlist
*/
BaseEditorDocumentParser::BaseEditorDocumentParser(const QString &filePath)
: m_mutex(QMutex::Recursive)
, m_filePath(filePath)
, m_usePrecompiledHeaders(false)
, m_editorDefinesChangedSinceLastUpdate(false)
: m_filePath(filePath)
{
}
......@@ -65,44 +69,33 @@ QString BaseEditorDocumentParser::filePath() const
return m_filePath;
}
ProjectPart::Ptr BaseEditorDocumentParser::projectPart() const
{
QMutexLocker locker(&m_mutex);
return m_projectPart;
}
void BaseEditorDocumentParser::setProjectPart(ProjectPart::Ptr projectPart)
BaseEditorDocumentParser::Configuration BaseEditorDocumentParser::configuration() const
{
QMutexLocker locker(&m_mutex);
m_manuallySetProjectPart = projectPart;
QMutexLocker locker(&m_stateAndConfigurationMutex);
return m_configuration;
}
bool BaseEditorDocumentParser::usePrecompiledHeaders() const
void BaseEditorDocumentParser::setConfiguration(const Configuration &configuration)
{
QMutexLocker locker(&m_mutex);
return m_usePrecompiledHeaders;
QMutexLocker locker(&m_stateAndConfigurationMutex);
m_configuration = configuration;
}
void BaseEditorDocumentParser::setUsePrecompiledHeaders(bool usePrecompiledHeaders)
BaseEditorDocumentParser::State BaseEditorDocumentParser::state() const
{
QMutexLocker locker(&m_mutex);
m_usePrecompiledHeaders = usePrecompiledHeaders;
QMutexLocker locker(&m_stateAndConfigurationMutex);
return m_state;
}
QByteArray BaseEditorDocumentParser::editorDefines() const
void BaseEditorDocumentParser::setState(const State &state)
{
QMutexLocker locker(&m_mutex);
return m_editorDefines;
QMutexLocker locker(&m_stateAndConfigurationMutex);
m_state = state;
}
void BaseEditorDocumentParser::setEditorDefines(const QByteArray &editorDefines)
ProjectPart::Ptr BaseEditorDocumentParser::projectPart() const
{
QMutexLocker locker(&m_mutex);
if (editorDefines != m_editorDefines) {
m_editorDefines = editorDefines;
m_editorDefinesChangedSinceLastUpdate = true;
}
return state().projectPart;
}
BaseEditorDocumentParser *BaseEditorDocumentParser::get(const QString &filePath)
......@@ -115,15 +108,17 @@ BaseEditorDocumentParser *BaseEditorDocumentParser::get(const QString &filePath)
return 0;
}
ProjectPart::Ptr BaseEditorDocumentParser::determineProjectPart() const
ProjectPart::Ptr BaseEditorDocumentParser::determineProjectPart(const QString &filePath,
const Configuration &config,
const State &state)
{
if (m_manuallySetProjectPart)
return m_manuallySetProjectPart;
if (config.manuallySetProjectPart)
return config.manuallySetProjectPart;
ProjectPart::Ptr projectPart = m_projectPart;
ProjectPart::Ptr projectPart = state.projectPart;
CppModelManager *cmm = CppModelManager::instance();
QList<ProjectPart::Ptr> projectParts = cmm->projectPart(m_filePath);
QList<ProjectPart::Ptr> projectParts = cmm->projectPart(filePath);
if (projectParts.isEmpty()) {
if (projectPart)
// File is not directly part of any project, but we got one before. We will re-use it,
......@@ -131,7 +126,7 @@ ProjectPart::Ptr BaseEditorDocumentParser::determineProjectPart() const
return projectPart;
// Fall-back step 1: Get some parts through the dependency table:
projectParts = cmm->projectPartFromDependencies(Utils::FileName::fromString(m_filePath));
projectParts = cmm->projectPartFromDependencies(Utils::FileName::fromString(filePath));
if (projectParts.isEmpty())
// Fall-back step 2: Use fall-back part from the model manager:
projectPart = cmm->fallbackProjectPart();
......@@ -146,14 +141,4 @@ ProjectPart::Ptr BaseEditorDocumentParser::determineProjectPart() const
return projectPart;
}
bool BaseEditorDocumentParser::editorDefinesChanged() const
{
return m_editorDefinesChangedSinceLastUpdate;
}
void BaseEditorDocumentParser::resetEditorDefinesChanged()
{
m_editorDefinesChangedSinceLastUpdate = false;
}
} // namespace CppTools
......@@ -42,45 +42,49 @@ class CPPTOOLS_EXPORT BaseEditorDocumentParser : public QObject
{
Q_OBJECT
public:
static BaseEditorDocumentParser *get(const QString &filePath);
struct Configuration {
bool usePrecompiledHeaders = false;
QByteArray editorDefines;
ProjectPart::Ptr manuallySetProjectPart;
};
public:
BaseEditorDocumentParser(const QString &filePath);
virtual ~BaseEditorDocumentParser();
QString filePath() const;
Configuration configuration() const;
void setConfiguration(const Configuration &configuration);
virtual void update(WorkingCopy workingCopy) = 0;
ProjectPart::Ptr projectPart() const;
void setProjectPart(ProjectPart::Ptr projectPart);
bool usePrecompiledHeaders() const;
void setUsePrecompiledHeaders(bool usePrecompiledHeaders);
QByteArray editorDefines() const;
void setEditorDefines(const QByteArray &editorDefines);
public:
static BaseEditorDocumentParser *get(const QString &filePath);
protected:
ProjectPart::Ptr determineProjectPart() const;
struct State {
QByteArray editorDefines;
ProjectPart::Ptr projectPart;
};
bool editorDefinesChanged() const;
void resetEditorDefinesChanged();
State state() const;
void setState(const State &state);
protected:
mutable QMutex m_mutex;
ProjectPart::Ptr m_projectPart;
static ProjectPart::Ptr determineProjectPart(const QString &filePath,
const Configuration &config,
const State &state);
mutable QMutex m_updateIsRunning;
mutable QMutex m_stateAndConfigurationMutex;
private:
const QString m_filePath;
ProjectPart::Ptr m_manuallySetProjectPart;
bool m_usePrecompiledHeaders;
QByteArray m_editorDefines;
bool m_editorDefinesChangedSinceLastUpdate;
Configuration m_configuration;
State m_state;
};
} // namespace CppTools
......
......@@ -37,7 +37,6 @@
#include <cplusplus/CppDocument.h>
#include <QMutex>
#include <QString>
namespace CppTools {
......@@ -49,14 +48,16 @@ class CPPTOOLS_EXPORT BuiltinEditorDocumentParser : public BaseEditorDocumentPar
public:
BuiltinEditorDocumentParser(const QString &filePath);
bool releaseSourceAndAST() const;
void setReleaseSourceAndAST(bool release);
void update(WorkingCopy workingCopy) override;
void releaseResources();
CPlusPlus::Document::Ptr document() const;
CPlusPlus::Snapshot snapshot() const;
ProjectPart::HeaderPaths headerPaths() const;
void setReleaseSourceAndAST(bool onoff);
void releaseResources();
signals:
void finished(CPlusPlus::Document::Ptr document, CPlusPlus::Snapshot snapshot);
......@@ -65,18 +66,26 @@ public:
static BuiltinEditorDocumentParser *get(const QString &filePath);
private:
void addFileAndDependencies(QSet<Utils::FileName> *toRemove, const Utils::FileName &fileName) const;
void addFileAndDependencies(CPlusPlus::Snapshot *snapshot,
QSet<Utils::FileName> *toRemove,
const Utils::FileName &fileName) const;
private:
QByteArray m_configFile;
struct ExtraState {
QByteArray configFile;
ProjectPart::HeaderPaths headerPaths;
QString projectConfigFile;
QStringList precompiledHeaders;
CPlusPlus::Snapshot snapshot;
bool forceSnapshotInvalidation = false;
};
ProjectPart::HeaderPaths m_headerPaths;
QString m_projectConfigFile;
QStringList m_precompiledHeaders;
ExtraState extraState() const;
void setExtraState(const ExtraState &extraState);
CPlusPlus::Snapshot m_snapshot;
bool m_forceSnapshotInvalidation;
bool m_releaseSourceAndAST;
bool m_releaseSourceAndAST = true;
ExtraState m_extraState;
};
} // namespace CppTools
......
......@@ -134,7 +134,10 @@ BuiltinEditorDocumentProcessor::BuiltinEditorDocumentProcessor(
using namespace Internal;
QSharedPointer<CppCodeModelSettings> cms = CppToolsPlugin::instance()->codeModelSettings();
m_parser.setUsePrecompiledHeaders(cms->pchUsage() != CppCodeModelSettings::PchUse_None);
BaseEditorDocumentParser::Configuration config = m_parser.configuration();
config.usePrecompiledHeaders = cms->pchUsage() != CppCodeModelSettings::PchUse_None;
m_parser.setConfiguration(config);
if (m_semanticHighlighter) {
m_semanticHighlighter->setHighlightingRunner(
......
......@@ -167,13 +167,11 @@ private:
const QString &m_filePath;
};
void waitForProcessedEditorDocument(const QString &filePath)
ProjectPart::Ptr projectPartOfEditorDocument(const QString &filePath)
{
CppEditorDocumentHandle *editorDocument
= CppModelManager::instance()->cppEditorDocument(filePath);
QVERIFY(editorDocument);
while (editorDocument->processor()->isParserRunning())
QCoreApplication::processEvents();
auto *editorDocument = CppModelManager::instance()->cppEditorDocument(filePath);
QTC_ASSERT(editorDocument, return ProjectPart::Ptr());
return editorDocument->processor()->parser()->projectPart();
}
} // anonymous namespace
......@@ -914,7 +912,9 @@ void CppToolsPlugin::test_modelmanager_precompiled_headers()
auto *parser = BuiltinEditorDocumentParser::get(fileName);
QVERIFY(parser);
parser->setUsePrecompiledHeaders(true);
BaseEditorDocumentParser::Configuration config = parser->configuration();
config.usePrecompiledHeaders = true;
parser->setConfiguration(config);
parser->update(mm->workingCopy());
// Check if defines from pch are considered
......@@ -995,7 +995,10 @@ void CppToolsPlugin::test_modelmanager_defines_per_editor()
const QString filePath = editor->document()->filePath().toString();
BaseEditorDocumentParser *parser = BaseEditorDocumentParser::get(filePath);
parser->setEditorDefines(editorDefines.toUtf8());
BaseEditorDocumentParser::Configuration config = parser->configuration();
config.editorDefines = editorDefines.toUtf8();
parser->setConfiguration(config);
parser->update(mm->workingCopy());
Document::Ptr doc = mm->document(main1File);
......@@ -1006,7 +1009,6 @@ void CppToolsPlugin::test_modelmanager_defines_per_editor()
void CppToolsPlugin::test_modelmanager_updateEditorsAfterProjectUpdate()
{
ModelManagerTestHelper helper;
CppModelManager *mm = CppModelManager::instance();
MyTestDataDir testDataDirectory(_("testdata_defines"));
const QString fileA = testDataDirectory.file(_("main1.cpp")); // content not relevant
......@@ -1017,10 +1019,8 @@ void CppToolsPlugin::test_modelmanager_updateEditorsAfterProjectUpdate()
QVERIFY(editorA);
EditorCloser closerA(editorA);
QCOMPARE(Core::DocumentModel::openedDocuments().size(), 1);
CppEditorDocumentHandle *editorDocumentA = mm->cppEditorDocument(fileA);
QVERIFY(editorDocumentA);
ProjectPart::Ptr documentAProjectPart = editorDocumentA->processor()->parser()->projectPart();
QVERIFY(TestCase::waitForProcessedEditorDocument(fileA));
ProjectPart::Ptr documentAProjectPart = projectPartOfEditorDocument(fileA);
QVERIFY(!documentAProjectPart->project);
// Open file B in editor
......@@ -1028,10 +1028,8 @@ void CppToolsPlugin::test_modelmanager_updateEditorsAfterProjectUpdate()
QVERIFY(editorB);
EditorCloser closerB(editorB);
QCOMPARE(Core::DocumentModel::openedDocuments().size(), 2);
CppEditorDocumentHandle *editorDocumentB = mm->cppEditorDocument(fileB);
QVERIFY(editorDocumentB);
ProjectPart::Ptr documentBProjectPart = editorDocumentB->processor()->parser()->projectPart();
QVERIFY(TestCase::waitForProcessedEditorDocument(fileB));
ProjectPart::Ptr documentBProjectPart = projectPartOfEditorDocument(fileB);
QVERIFY(!documentBProjectPart->project);
// Switch back to document A
......@@ -1053,16 +1051,14 @@ void CppToolsPlugin::test_modelmanager_updateEditorsAfterProjectUpdate()
helper.updateProjectInfo(pi);
// ... and check for updated editor document A
while (editorDocumentA->processor()->isParserRunning())
QCoreApplication::processEvents();
documentAProjectPart = editorDocumentA->processor()->parser()->projectPart();
QVERIFY(TestCase::waitForProcessedEditorDocument(fileA));
documentAProjectPart = projectPartOfEditorDocument(fileA);
QCOMPARE(documentAProjectPart->project, project);
// Switch back to document B and check if that's updated, too
Core::EditorManager::activateEditor(editorB);
while (editorDocumentB->processor()->isParserRunning())
QCoreApplication::processEvents();
documentBProjectPart = editorDocumentB->processor()->parser()->projectPart();
QVERIFY(TestCase::waitForProcessedEditorDocument(fileB));
documentBProjectPart = projectPartOfEditorDocument(fileB);
QCOMPARE(documentBProjectPart->project, project);
}
......@@ -1131,7 +1127,7 @@ void CppToolsPlugin::test_modelmanager_documentsAndRevisions()
TextEditor::BaseTextEditor *editor1;
QVERIFY(helper.openBaseTextEditor(filePath1, &editor1));
helper.closeEditorAtEndOfTestCase(editor1);
waitForProcessedEditorDocument(filePath1);
QVERIFY(TestCase::waitForProcessedEditorDocument(filePath1));
VERIFY_DOCUMENT_REVISION(modelManager->document(filePath1), 2U);
VERIFY_DOCUMENT_REVISION(modelManager->document(filePath2), 1U);
......@@ -1144,7 +1140,7 @@ void CppToolsPlugin::test_modelmanager_documentsAndRevisions()
TextEditor::BaseTextEditor *editor2;
QVERIFY(helper.openBaseTextEditor(filePath2, &editor2));
helper.closeEditorAtEndOfTestCase(editor2);
waitForProcessedEditorDocument(filePath2);
QVERIFY(TestCase::waitForProcessedEditorDocument(filePath2));
VERIFY_DOCUMENT_REVISION(modelManager->document(filePath1), 3U);
VERIFY_DOCUMENT_REVISION(modelManager->document(filePath2), 3U);
......
......@@ -137,6 +137,7 @@ void CppToolsPlugin::test_cppsourceprocessor_includes_cyclic()
const QString filePath = editor->document()->filePath().toString();
auto *processor = BaseEditorDocumentProcessor::get(filePath);
QVERIFY(processor);
QVERIFY(TestCase::waitForProcessedEditorDocument(filePath));
Snapshot snapshot = processor->snapshot();
QCOMPARE(snapshot.size(), 3); // Configuration file included
......
......@@ -29,6 +29,10 @@
****************************************************************************/
#include "cpptoolstestcase.h"
#include "baseeditordocumentparser.h"
#include "baseeditordocumentprocessor.h"
#include "editordocumenthandle.h"
#include "cppmodelmanager.h"
#include "cppworkingcopy.h"
......@@ -140,6 +144,31 @@ bool TestCase::garbageCollectGlobalSnapshot()
return globalSnapshot().isEmpty();
}
static bool waitForProcessedEditorDocument_internal(CppEditorDocumentHandle *editorDocument,
int timeOutInMs)
{
QTC_ASSERT(editorDocument, return false);
QTime timer;
timer.start();
forever {
if (!editorDocument->processor()->isParserRunning())
return true;
if (timer.elapsed() > timeOutInMs)
return false;
QCoreApplication::processEvents();
QThread::msleep(20);
}
}
bool TestCase::waitForProcessedEditorDocument(const QString &filePath, int timeOutInMs)
{
auto *editorDocument = CppModelManager::instance()->cppEditorDocument(filePath);
return waitForProcessedEditorDocument_internal(editorDocument, timeOutInMs);
}
bool TestCase::parseFiles(const QSet<QString> &filePaths)
{
CppModelManager::instance()->updateSourceFiles(filePaths).waitForFinished();
......
......@@ -95,6 +95,8 @@ public:
static CPlusPlus::Snapshot globalSnapshot();
static bool garbageCollectGlobalSnapshot();
static bool waitForProcessedEditorDocument(const QString &filePath, int timeOutInMs = 5000);
enum { defaultTimeOutInMs = 30 * 1000 /*= 30 secs*/ };
static bool waitUntilCppModelManagerIsAwareOf(
ProjectExplorer::Project *project,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment