[kdevelop/kdevelop] kdevplatform/language/duchain: DUChain: Implement queued loading
Jarmo Tiitto <[email protected]>
| Newsgroups | gmane.comp.kde.cvs |
|---|---|
| Message-ID | <[email protected]> |
Git commit e21148aeb1d429360902f776a397d0a078d0b81e by Jarmo Tiitto.
Committed on 02/08/2026 at 13:13.
Pushed by jatothrim into branch 'master'.
DUChain: Implement queued loading
Currently, DUChain::loadChain() processing does not cooperate with the
other threads, and on conflict a QThread::usleep(50000) wait might
happen. This function seems haphazardly put together because the
"QSet<uint>& loaded" parameter neither provides benefit in ensuring two
conflicting loads are not attempted at same time.
Therefore, rewrite the DUChain::loadChain() and only rely on the
DUChain::chainsByIndex array to tell if a top context is currently
loaded. Implement a loading queue so that loadChain() does not need to
call itself recursively and such that other threads can cooperate on the
loading. For each importedParentContexts() index the
rebuildDynamicImportStructure() is now called after the top-context that
caused loading it, rather than before. This specific change does not
seem have any ill-effects, although this majorly differs from the
original recursive way.
Locking both chainsByIndexLock and m_chainsMutex mutexes requires that
m_chainsMutex is never acquired after chainsByIndexLock. The callers of
loadChain() should not do a check via readChainForIndex() or
hasChainForIndex() before calling loadChain(), because it is much more
efficient to acquire chainsByIndexLock just once and return the
top-context from loadChain(). This also lessens the risk of a dead-lock
due to locking-order inversion.
M +7 -14 kdevplatform/language/duchain/duchain.cpp
M +3 -12 kdevplatform/language/duchain/duchain.h
M +93 -25 kdevplatform/language/duchain/duchainprivate.cpp
M +19 -5 kdevplatform/language/duchain/duchainprivate.h
https://invent.kde.org/kdevelop/kdevelop/-/commit/e21148aeb1d429360902f776a397d0a078d0b81e
diff --git a/kdevplatform/language/duchain/duchain.cpp b/kdevplatform/language/duchain/duchain.cpp
index 26b1bb88a2..16e26b4183 100644
--- a/kdevplatform/language/duchain/duchain.cpp
+++ b/kdevplatform/language/duchain/duchain.cpp
@@ -210,6 +210,12 @@ void DUChain::addDocumentChain(TopDUContext* chain)
chain->setInDuChain(true);
+ // Remove this chain from m_loadingSet and m_pendingLoads.
+ sdDUChainPrivate->m_pendingLoads.removeIf([index = chain->ownIndex()](const auto& context) -> bool {
+ return index == context.index;
+ });
+ sdDUChainPrivate->m_loadingSet.remove(chain->ownIndex());
+
l.unlock();
addToEnvironmentManager(chain);
@@ -275,20 +281,7 @@ IndexedString DUChain::urlForIndex(uint index) const
TopDUContext* DUChain::loadChain(uint index)
{
- QSet<uint> loaded;
- sdDUChainPrivate->loadChain(index, loaded);
-
- {
- QMutexLocker lock(&chainsByIndexLock);
-
- if (chainsByIndex.size() > index) {
- TopDUContext* top = chainsByIndex[index];
- if (top)
- return top;
- }
- }
-
- return nullptr;
+ return sdDUChainPrivate->loadChain(index);
}
TopDUContext* DUChain::chainForDocument(const KDevelop::IndexedString& document, bool proxyContext) const
diff --git a/kdevplatform/language/duchain/duchain.h b/kdevplatform/language/duchain/duchain.h
index 244f40ed1e..13b10bb944 100644
--- a/kdevplatform/language/duchain/duchain.h
+++ b/kdevplatform/language/duchain/duchain.h
@@ -167,17 +167,7 @@ public:
if (m_deleted)
return nullptr;
- {
- QMutexLocker lock(&chainsByIndexLock);
-
- if (chainsByIndex.size() > index) {
- TopDUContext* top = chainsByIndex[index];
- if (top)
- return top;
- }
- }
-
- //Load the top-context
+ // Load or get the top-context
return loadChain(index);
}
@@ -283,10 +273,11 @@ private Q_SLOTS:
void documentClosed(KDevelop::IDocument*);
private:
- TopDUContext* loadChain(uint index);
+ [[nodiscard]] TopDUContext* loadChain(uint index);
//These two are exported here so that the extremely frequently called chainForIndex(..) can be inlined
static bool m_deleted;
static std::vector<TopDUContext*> chainsByIndex;
+ // @warn while chainsByIndexLock is acquired DUChainPrivate::m_chainsMutex must not be released.
static QMutex chainsByIndexLock;
/// Increases the reference-count for the given top-context. The result: It will not be unloaded.
diff --git a/kdevplatform/language/duchain/duchainprivate.cpp b/kdevplatform/language/duchain/duchainprivate.cpp
index 30bd65f378..ec93c2bbe1 100644
--- a/kdevplatform/language/duchain/duchainprivate.cpp
+++ b/kdevplatform/language/duchain/duchainprivate.cpp
@@ -7,6 +7,8 @@
#include "duchainprivate.h"
+#include <util/foregroundlock.h>
+
namespace KDevelop {
//This thing is not actually used, but it's needed for compiling
@@ -248,14 +250,12 @@ DEFINE_LIST_MEMBER_HASH(EnvironmentInformationListItem, items, uint)
return ret;
}
- ///Must be called _without_ the chainsByIndex spin-lock locked
bool DUChainPrivate::hasChainForIndex(uint index)
{
QMutexLocker lock(&DUChain::chainsByIndexLock);
return (DUChain::chainsByIndex.size() > index) && DUChain::chainsByIndex[index];
}
- ///Must be called _without_ the chainsByIndex spin-lock locked. Returns the top-context if it is loaded.
TopDUContext* DUChainPrivate::readChainForIndex(uint index)
{
QMutexLocker lock(&DUChain::chainsByIndexLock);
@@ -265,49 +265,117 @@ DEFINE_LIST_MEMBER_HASH(EnvironmentInformationListItem, items, uint)
return nullptr;
}
- ///Makes sure that the chain with the given index is loaded
- ///@warning m_chainsMutex must NOT be locked when this is called
- void DUChainPrivate::loadChain(uint index, QSet<uint>& loaded)
+ TopDUContext* DUChainPrivate::loadChain(uint index)
{
QMutexLocker l(&m_chainsMutex);
- if (!hasChainForIndex(index)) {
- if (!Algorithm::insert(m_loading, index).inserted) {
- //It's probably being loaded by another thread. So wait until the load is ready
- while (m_loading.contains(index)) {
+ if (auto chain = readChainForIndex(index)) {
+ // Already loaded.
+ return chain;
+ }
+
+ // Try place the index into the loading queue.
+ if (m_loadingSet.find(index) == m_loadingSet.end()) {
+ m_loadingSet.insert(index, LoadStatus::Pending);
+ m_pendingLoads.emplace_back(LoadChainOp{index});
+ }
+
+ auto checkIfDone = [this](uint index) -> QPair<bool, TopDUContext*> {
+ auto it = m_loadingSet.find(index);
+ auto chain = readChainForIndex(index);
+ if (it == m_loadingSet.end() && chain) {
+ // Success.
+ return {true, chain};
+ } else if (it != m_loadingSet.end() && it.value() == LoadStatus::Failed) {
+ // Failed to load.
+ m_pendingLoads.removeIf([index](const auto& context) -> bool {
+ return index == context.index;
+ });
+ m_loadingSet.erase(it);
+ return {true, nullptr};
+ }
+ return {false, nullptr};
+ };
+
+ // Load top contexts until our chain appears in chainsByIndex and is removed from m_loadingSet.
+ for (;;) {
+ auto ready = checkIfDone(index);
+ if (ready.first) {
+ return ready.second;
+ }
+
+ while (m_pendingLoads.isEmpty()) {
+ l.unlock();
+ {
+ // Release the foreground mutex while yielding.
+ TemporarilyReleaseForegroundLock unlocker;
+ QThread::yieldCurrentThread();
+ }
+ l.relock();
+
+ ready = checkIfDone(index);
+ if (ready.first) {
+ return ready.second;
+ }
+ }
+
+ const auto context = m_pendingLoads.takeFirst();
+ if (context.chain) {
+ if (!hasChainForIndex(context.index)) {
l.unlock();
- qCDebug(LANGUAGE) << "waiting for another thread to load index" << index;
- QThread::usleep(50000);
+ // Finally add the chain.
+ context.chain->rebuildDynamicImportStructure();
+ context.chain->setInDuChain(true);
+ instance->addDocumentChain(context.chain);
+ if (context.index == index) {
+ // Task accomplished.
+ return context.chain;
+ }
l.relock();
}
- loaded.insert(index);
- return;
+ continue;
}
- loaded.insert(index);
l.unlock();
- qCDebug(LANGUAGE) << "loading top-context" << index;
- TopDUContext* chain = TopDUContextDynamicData::load(index);
+
+ if (context.index != index) {
+ qCDebug(LANGUAGE) << "helping loading top-context" << context.index;
+ } else {
+ qCDebug(LANGUAGE) << "loading top-context" << index;
+ }
+
+ TopDUContext* chain = TopDUContextDynamicData::load(context.index);
if (chain) {
chain->setParsingEnvironmentFile(loadInformation(chain->ownIndex()));
if (!chain->usingImportsCache()) {
//Eventually also load all the imported chains, so the import-structure is built
const auto importedParentContexts = chain->DUContext::importedParentContexts();
+ l.relock();
for (const DUContext::Import& import : importedParentContexts) {
- if (!loaded.contains(import.topContextIndex())) {
- loadChain(import.topContextIndex(), loaded);
+ const auto importIndex = import.topContextIndex();
+
+ if (hasChainForIndex(importIndex)) {
+ // Already loaded.
+ continue;
+ }
+
+ if (m_loadingSet.find(importIndex) == m_loadingSet.end()) {
+ m_loadingSet.insert(importIndex, LoadStatus::Pending);
+ m_pendingLoads.emplace_back(LoadChainOp{importIndex});
}
}
+ l.unlock();
}
- chain->rebuildDynamicImportStructure();
-
- chain->setInDuChain(true);
- instance->addDocumentChain(chain);
+ // Eventually add the chain to chainsByIndex.
+ // Only one thread can succeed in this, but the request can be enqueued multiple times.
+ l.relock();
+ m_pendingLoads.emplace_back(LoadChainOp{context.index, chain});
+ } else {
+ l.relock();
+ // Failed to load.
+ m_loadingSet[context.index] = LoadStatus::Failed;
}
-
- l.relock();
- m_loading.remove(index);
}
}
diff --git a/kdevplatform/language/duchain/duchainprivate.h b/kdevplatform/language/duchain/duchainprivate.h
index d730975d2c..ef5a9f0e9d 100644
--- a/kdevplatform/language/duchain/duchainprivate.h
+++ b/kdevplatform/language/duchain/duchainprivate.h
@@ -386,7 +386,19 @@ public:
Definitions m_definitions;
Uses m_uses;
- QSet<uint> m_loading;
+ // Unique set of indices being loaded.
+ enum class LoadStatus : bool {
+ Pending = true,
+ Failed = false
+ };
+ QHash<uint, LoadStatus> m_loadingSet;
+ // The loading queue.
+ struct LoadChainOp
+ {
+ uint index;
+ TopDUContext* chain = nullptr;
+ };
+ QList<LoadChainOp> m_pendingLoads;
bool m_cleanupDisabled;
//List of available top-context indices, protected by m_chainsMutex
@@ -408,15 +420,17 @@ public:
///m_chainsMutex should _not_ be locked, because this may trigger I/O
QList<ParsingEnvironmentFilePointer> getEnvironmentInformation(const IndexedString& url);
- ///Must be called _without_ the chainsByIndex spin-lock locked
+ ///Must be called _without_ the chainsByIndex locked
static bool hasChainForIndex(uint index);
- ///Must be called _without_ the chainsByIndex spin-lock locked. Returns the top-context if it is loaded.
+ ///Must be called _without_ the chainsByIndex locked. Returns the top-context if it is loaded.
static TopDUContext* readChainForIndex(uint index);
- ///Makes sure that the chain with the given index is loaded
+ ///Makes sure that the chain with the given index is loaded if possible.
+ ///Other chains than the requested one maybe loaded to cooperate with other threads.
///@warning m_chainsMutex must NOT be locked when this is called
- void loadChain(uint index, QSet<uint>& loaded);
+ ///@return Returns the top-context or nullptr if it was not possible to load the given index.
+ [[nodiscard]] TopDUContext* loadChain(uint index);
///Stores all environment-information
///Also makes sure that all information that stays is referenced, so it stays alive.