Commit 5ee9b9fe authored by Kai Stierand's avatar Kai Stierand Committed by Milian Wolff
Browse files

Improve performance when queueing documents for parsing

I'm working with a project that has more than 1400 Unit-Tests and they are all in a single executable. This causes CTestFindJob to queue each c++ File compiled into the test executable more than 1400 times. We might probably want to change something about that, but that is a bit more work; Therefore I chose to get the low hanging fruits. These changes improve the queuing performance by a factor of approx. 10. For my project this means roughly 3 Seconds of waiting instead of more than 30 Seconds before the change.
parent bcbeaf26
Pipeline #96096 canceled with stage
......@@ -110,39 +110,27 @@ inline uint qHash(const DocumentParseTarget& target)
+ static_cast<uint>(reinterpret_cast<quintptr>(target.notifyWhenReady.data()));
}
struct DocumentParsePlan
class DocumentParsePlan
{
QSet<DocumentParseTarget> targets;
public:
ParseJob::SequentialProcessingFlags sequentialProcessingFlags() const
{
//Pick the strictest possible flags
ParseJob::SequentialProcessingFlags ret = ParseJob::IgnoresSequentialProcessing;
for (const DocumentParseTarget& target : targets) {
for (const DocumentParseTarget& target : m_targets) {
ret |= target.sequentialProcessingFlags;
}
return ret;
}
int priority() const
{
//Pick the best priority
int ret = BackgroundParser::WorstPriority;
for (const DocumentParseTarget& target : targets) {
if (target.priority < ret) {
ret = target.priority;
}
}
return ret;
}
int priority() const { return m_priority; }
TopDUContext::Features features() const
{
//Pick the best features
TopDUContext::Features ret{};
for (const DocumentParseTarget& target : targets) {
for (const DocumentParseTarget& target : m_targets) {
ret |= target.features;
}
......@@ -153,13 +141,45 @@ struct DocumentParsePlan
{
QVector<QPointer<QObject>> ret;
for (const DocumentParseTarget& target : targets) {
for (const DocumentParseTarget& target : m_targets) {
if (target.notifyWhenReady)
ret << target.notifyWhenReady;
}
return ret;
}
const QSet<const DocumentParseTarget>& targets() const
{
return m_targets;
}
void addTarget(const DocumentParseTarget& target)
{
if (target.priority < m_priority) {
m_priority = target.priority;
}
m_targets.insert(target);
}
void removeTargetsForListener(QObject* notifyWhenReady)
{
m_priority = BackgroundParser::WorstPriority;
for (auto it = m_targets.cbegin(), end = m_targets.cend(); it != end;) {
if (it->notifyWhenReady.data() == notifyWhenReady) {
it = m_targets.erase(it);
} else {
if (it->priority < m_priority) {
m_priority = it->priority;
}
++it;
}
}
}
private:
QSet<const DocumentParseTarget> m_targets;
int m_priority = BackgroundParser::WorstPriority;
};
Q_DECLARE_TYPEINFO(DocumentParseTarget, Q_MOVABLE_TYPE);
......@@ -168,6 +188,13 @@ Q_DECLARE_TYPEINFO(DocumentParsePlan, Q_MOVABLE_TYPE);
class KDevelop::BackgroundParserPrivate
{
public:
enum class AddBehavior
{
AddIfMissing,
OnlyUpdateExisting,
};
BackgroundParserPrivate(BackgroundParser* parser, ILanguageController* languageController)
: m_parser(parser)
, m_languageController(languageController)
......@@ -299,7 +326,7 @@ public:
const auto parsePlanIt = m_documents.find(url);
if (parsePlanIt != m_documents.end()) {
// Remove all mentions of this document.
for (const auto& target : qAsConst(parsePlanIt->targets)) {
for (const auto& target : parsePlanIt->targets()) {
m_documentsForPriority[target.priority].remove(url);
}
......@@ -452,6 +479,50 @@ public:
m_weaver.resume();
}
bool addDocumentListener(const IndexedString& url, TopDUContext::Features features, int priority,
QObject* notifyWhenReady, ParseJob::SequentialProcessingFlags flags, int delay,
AddBehavior addBehavior)
{
qCDebug(LANGUAGE) << "BackgroundParserPrivate::addDocumentListener" << url << url.toUrl();
Q_ASSERT(isValidURL(url));
DocumentParseTarget target;
target.priority = priority;
target.features = features;
target.sequentialProcessingFlags = flags;
target.notifyWhenReady = QPointer<QObject>(notifyWhenReady);
{
QMutexLocker lock(&m_mutex);
auto it = m_documents.find(url);
if (it != m_documents.end()) {
//Update the stored plan
auto currentPrio = it.value().priority();
it.value().addTarget(target);
if (currentPrio > target.priority) {
m_documentsForPriority[currentPrio].remove(url);
m_documentsForPriority[target.priority].insert(url);
}
} else if (addBehavior == AddBehavior::AddIfMissing) {
// qCDebug(LANGUAGE) << "BackgroundParser::addDocument: queuing" << cleanedUrl;
auto& doc = m_documents[url];
doc.addTarget(target);
m_documentsForPriority[doc.priority()].insert(url);
++m_maxParseJobs; //So the progress-bar waits for this document
} else {
return false;
}
if (delay == ILanguageSupport::DefaultDelay) {
delay = m_delay;
}
}
startTimerThreadSafe(delay);
return true;
}
BackgroundParser* m_parser;
ILanguageController* m_languageController;
......@@ -558,20 +629,16 @@ void BackgroundParser::parseProgress(KDevelop::ParseJob* job, float value, const
void BackgroundParser::revertAllRequests(QObject* notifyWhenReady)
{
Q_ASSERT(notifyWhenReady != nullptr);
Q_D(BackgroundParser);
QMutexLocker lock(&d->m_mutex);
for (auto it = d->m_documents.begin(); it != d->m_documents.end();) {
d->m_documentsForPriority[it.value().priority()].remove(it.key());
const auto oldTargets = (*it).targets;
for (const DocumentParseTarget& target : oldTargets) {
if (notifyWhenReady && target.notifyWhenReady.data() == notifyWhenReady) {
(*it).targets.remove(target);
}
}
it->removeTargetsForListener(notifyWhenReady);
if ((*it).targets.isEmpty()) {
if ((*it).targets().isEmpty()) {
it = d->m_documents.erase(it);
--d->m_maxParseJobs;
......@@ -583,41 +650,21 @@ void BackgroundParser::revertAllRequests(QObject* notifyWhenReady)
}
}
bool BackgroundParser::addListenerToDocumentIfExist(const IndexedString& url, TopDUContext::Features features, int priority,
QObject* notifyWhenReady, ParseJob::SequentialProcessingFlags flags,
int delay)
{
Q_D(BackgroundParser);
return d->addDocumentListener(url, features, priority, notifyWhenReady, flags, delay,
BackgroundParserPrivate::AddBehavior::OnlyUpdateExisting);
}
void BackgroundParser::addDocument(const IndexedString& url, TopDUContext::Features features, int priority,
QObject* notifyWhenReady, ParseJob::SequentialProcessingFlags flags, int delay)
{
Q_D(BackgroundParser);
qCDebug(LANGUAGE) << "BackgroundParser::addDocument" << url << url.toUrl();
Q_ASSERT(isValidURL(url));
QMutexLocker lock(&d->m_mutex);
{
DocumentParseTarget target;
target.priority = priority;
target.features = features;
target.sequentialProcessingFlags = flags;
target.notifyWhenReady = QPointer<QObject>(notifyWhenReady);
auto it = d->m_documents.find(url);
if (it != d->m_documents.end()) {
//Update the stored plan
d->m_documentsForPriority[it.value().priority()].remove(url);
it.value().targets << target;
d->m_documentsForPriority[it.value().priority()].insert(url);
} else {
// qCDebug(LANGUAGE) << "BackgroundParser::addDocument: queuing" << cleanedUrl;
d->m_documents[url].targets << target;
d->m_documentsForPriority[d->m_documents[url].priority()].insert(url);
++d->m_maxParseJobs; //So the progress-bar waits for this document
}
if (delay == ILanguageSupport::DefaultDelay) {
delay = d->m_delay;
}
d->startTimerThreadSafe(delay);
}
d->addDocumentListener(url, features, priority, notifyWhenReady, flags, delay,
BackgroundParserPrivate::AddBehavior::AddIfMissing);
}
void BackgroundParser::removeDocument(const IndexedString& url, QObject* notifyWhenReady)
......@@ -633,14 +680,9 @@ void BackgroundParser::removeDocument(const IndexedString& url, QObject* notifyW
auto& documentParsePlan = *documentParsePlanIt;
d->m_documentsForPriority[documentParsePlan.priority()].remove(url);
const auto oldTargets = documentParsePlan.targets;
for (const DocumentParseTarget& target : oldTargets) {
if (target.notifyWhenReady.data() == notifyWhenReady) {
documentParsePlan.targets.remove(target);
}
}
documentParsePlan.removeTargetsForListener(notifyWhenReady);
if (documentParsePlan.targets.isEmpty()) {
if (documentParsePlan.targets().isEmpty()) {
d->m_documents.erase(documentParsePlanIt);
--d->m_maxParseJobs;
} else {
......
......@@ -141,7 +141,6 @@ public Q_SLOTS:
/// Reverts all requests that were made for the given notification-target.
/// priorities and requested features will be reverted as well.
/// When @p notifyWhenReady is set to a nullptr, all requests will be reverted.
void revertAllRequests(QObject* notifyWhenReady);
/**
......@@ -163,6 +162,19 @@ public Q_SLOTS:
ParseJob::SequentialProcessingFlags flags = ParseJob::IgnoresSequentialProcessing,
int delay_ms = ILanguageSupport::DefaultDelay);
/**
* try to add a listener to an existing document
* @see addDocument(...) for parameter description
* @returns true if the document existed and a listener was added
*/
[[nodiscard]]
bool addListenerToDocumentIfExist(const IndexedString& url,
TopDUContext::Features features = TopDUContext::VisibleDeclarationsAndContexts,
int priority = 0,
QObject* notifyWhenReady = nullptr,
ParseJob::SequentialProcessingFlags flags = ParseJob::IgnoresSequentialProcessing,
int delay_ms = ILanguageSupport::DefaultDelay);
/**
* Removes the @p url that is registered for the given notification from the url.
*
......
......@@ -1827,20 +1827,34 @@ KDevelop::ReferencedTopDUContext DUChain::waitForUpdate(const KDevelop::IndexedS
void DUChain::updateContextForUrl(const IndexedString& document, TopDUContext::Features minFeatures,
QObject* notifyReady, int priority) const
{
DUChainReadLocker lock(DUChain::lock());
TopDUContext* standardContext = DUChainUtils::standardContextForUrl(document.toUrl());
if (standardContext && standardContext->parsingEnvironmentFile() &&
!standardContext->parsingEnvironmentFile()->needsUpdate() &&
standardContext->parsingEnvironmentFile()->featuresSatisfied(minFeatures)) {
lock.unlock();
if (notifyReady)
QMetaObject::invokeMethod(notifyReady, "updateReady", Qt::DirectConnection,
Q_ARG(KDevelop::IndexedString, document),
Q_ARG(KDevelop::ReferencedTopDUContext, ReferencedTopDUContext(standardContext)));
} else {
///Start a parse-job for the given document
ICore::self()->languageController()->backgroundParser()->addDocument(document, minFeatures, priority,
notifyReady);
// the call to DUChainUtils::standardContextForUrl(...) takes surprisingly long
// however if a document is already scheduled for parsing, we know it needs parsing and can
// skip the expansive check.
auto* backgroundParser = ICore::self()->languageController()->backgroundParser();
// atomically check if the document is already scheduled and if so add a new listener:
bool listenerAdded = backgroundParser->addListenerToDocumentIfExist(document, minFeatures, priority, notifyReady);
// if the document isn't already scheduled and the listener was added:
if (!listenerAdded)
{
DUChainReadLocker lock(DUChain::lock());
// check if the document needs parsing
TopDUContext* standardContext = DUChainUtils::standardContextForUrl(document.toUrl());
if (standardContext && standardContext->parsingEnvironmentFile() &&
!standardContext->parsingEnvironmentFile()->needsUpdate() &&
standardContext->parsingEnvironmentFile()->featuresSatisfied(minFeatures)) {
// if the document doesn't need parsing we can immedistely return the already parsed context to the listener
lock.unlock();
if (notifyReady) {
// do not remove qualification KDevelop:: or invokeMethod will not find the proper method
QMetaObject::invokeMethod(notifyReady, "updateReady", Qt::DirectConnection,
Q_ARG(KDevelop::IndexedString, document),
Q_ARG(KDevelop::ReferencedTopDUContext, ReferencedTopDUContext(standardContext)));
}
} else {
///Start a parse-job for the given document
lock.unlock();
backgroundParser->addDocument(document, minFeatures, priority, notifyReady);
}
}
}
......
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