Commit 8c477a49 authored by Milian Wolff's avatar Milian Wolff
Browse files

Port duchain environment item repositories to non recursive mutices

Again, provide the mutex externally and lock it manually before
accessing the item repository API.

The code here is pretty ugly and brittle, but I hope I'm not making
things worse while touching it here. Testing shows that it all still
seems to work even on my 8 core / 16 thread laptop.
parent 02c00e17
......@@ -485,7 +485,10 @@ public:
void addEnvironmentInformation(ParsingEnvironmentFilePointer info)
{
Q_ASSERT(!findInformation(info->indexedTopContext().index()));
Q_ASSERT(m_environmentInfo.findIndex(info->indexedTopContext().index()) == 0);
Q_ASSERT([&]() {
QMutexLocker lock(&m_environmentInfoMutex);
return m_environmentInfo.findIndex(info->indexedTopContext().index()) == 0;
}());
QMutexLocker lock(&m_chainsMutex);
m_fileEnvironmentInformations.insert(info->url(), info);
......@@ -511,7 +514,7 @@ public:
{
//Remove it from the environment information lists if it was there
QMutexLocker lock(m_environmentListInfo.mutex());
QMutexLocker lock(&m_environmentListInfoMutex);
uint index = m_environmentListInfo.findIndex(info->url());
if (index) {
......@@ -524,7 +527,7 @@ public:
}
}
QMutexLocker lock(m_environmentInfo.mutex());
QMutexLocker lock(&m_environmentInfoMutex);
uint index = m_environmentInfo.findIndex(info->indexedTopContext().index());
if (index) {
m_environmentInfo.deleteItem(index);
......@@ -540,22 +543,25 @@ public:
QList<ParsingEnvironmentFilePointer> getEnvironmentInformation(const IndexedString& url)
{
QList<ParsingEnvironmentFilePointer> ret;
uint listIndex = m_environmentListInfo.findIndex(url);
if (listIndex) {
{
KDevVarLengthArray<uint> topContextIndices;
{
//First store all the possible indices into the KDevVarLengthArray, so we can unlock the mutex before processing them.
QMutexLocker lock(m_environmentListInfo.mutex()); //Lock the mutex to make sure the item isn't changed while it's being iterated
const EnvironmentInformationListItem* item = m_environmentListInfo.itemFromIndex(listIndex);
FOREACH_FUNCTION(uint topContextIndex, item->items)
topContextIndices << topContextIndex;
QMutexLocker lock(&m_environmentListInfoMutex); // Lock the mutex to make sure the item isn't changed
// while it's being iterated
const EnvironmentInformationListItem* item = m_environmentListInfo.findItem(url);
if (item) {
FOREACH_FUNCTION(uint topContextIndex, item->items)
topContextIndices << topContextIndex;
}
}
//Process the indices in a separate step after copying them from the array, so we don't need m_environmentListInfo.mutex locked,
//and can call loadInformation(..) safely, which else might lead to a deadlock.
// Process the indices in a separate step after copying them from the array, so we don't need
// m_environmentListInfoMutex locked, and can call loadInformation(..) safely, which else might lead to a
// deadlock.
for (uint topContextIndex : qAsConst(topContextIndices)) {
QExplicitlySharedDataPointer<ParsingEnvironmentFile> p =
ParsingEnvironmentFilePointer(loadInformation(topContextIndex));
......@@ -666,7 +672,7 @@ public:
for (const ParsingEnvironmentFilePointer& file : qAsConst(check)) {
EnvironmentInformationRequest req(file.data());
QMutexLocker lock(m_environmentInfo.mutex());
QMutexLocker lock(&m_environmentInfoMutex);
uint index = m_environmentInfo.findIndex(req);
if (file->d_func()->isDynamic()) {
......@@ -709,13 +715,17 @@ public:
storeInformationList(url);
//Access the data in the repository, so the bucket isn't unloaded
uint index = m_environmentListInfo.findIndex(EnvironmentInformationListRequest(url));
if (index) {
m_environmentListInfo.itemFromIndex(index);
} else {
QMutexLocker lock(&m_chainsMutex);
qCDebug(LANGUAGE) << "Did not find stored item for" << url.str() << "count:" <<
m_fileEnvironmentInformations.values(url);
{
QMutexLocker lock(&m_environmentListInfoMutex);
uint index = m_environmentListInfo.findIndex(EnvironmentInformationListRequest(url));
if (index) {
m_environmentListInfo.itemFromIndex(index);
} else {
lock.unlock();
QMutexLocker chainLock(&m_chainsMutex);
qCDebug(LANGUAGE) << "Did not find stored item for" << url.str()
<< "count:" << m_fileEnvironmentInformations.values(url);
}
}
if (!atomic) {
locker.unlock();
......@@ -992,14 +1002,16 @@ unloadContexts:
return alreadyLoaded;
//Step two: Check if it is on disk, and if is, load it
uint dataIndex = m_environmentInfo.findIndex(EnvironmentInformationRequest(topContextIndex));
if (!dataIndex) {
const EnvironmentInformationItem* item = nullptr;
{
QMutexLocker lock(&m_environmentInfoMutex);
item = m_environmentInfo.findItem(EnvironmentInformationRequest(topContextIndex));
}
if (!item) {
//No environment-information stored for this top-context
return nullptr;
}
const EnvironmentInformationItem& item(*m_environmentInfo.itemFromIndex(dataIndex));
QMutexLocker lock(&m_chainsMutex);
//Due to multi-threading, we must do this check after locking the mutex, so we can be sure we don't create the same item twice at the same time
......@@ -1008,16 +1020,9 @@ unloadContexts:
return alreadyLoaded;
///FIXME: ugly, and remove const_cast
auto* ret = dynamic_cast<ParsingEnvironmentFile*>(DUChainItemSystem::self().create(
const_cast<DUChainBaseData*>(
reinterpret_cast<const
DUChainBaseData*>(
reinterpret_cast<const char*>(&
item)
+
sizeof(
EnvironmentInformationItem)))
));
auto* ret = dynamic_cast<ParsingEnvironmentFile*>(
DUChainItemSystem::self().create(const_cast<DUChainBaseData*>(reinterpret_cast<const DUChainBaseData*>(
reinterpret_cast<const char*>(item) + sizeof(EnvironmentInformationItem)))));
if (ret) {
Q_ASSERT(ret->d_func()->classId);
Q_ASSERT(ret->indexedTopContext().index() == topContextIndex);
......@@ -1048,7 +1053,10 @@ unloadContexts:
qCDebug(LANGUAGE) << "cleaning top-contexts";
CleanupListVisitor visitor;
uint startPos = 0;
m_environmentInfo.visitAllItems(visitor);
{
QMutexLocker environmentInfoLock(&m_environmentInfoMutex);
m_environmentInfo.visitAllItems(visitor);
}
int checkContextsCount = maxFinalCleanupCheckContexts;
int percentageOfContexts = (visitor.checkContexts.size() * 100) / minimumFinalCleanupCheckContextsPercentage;
......@@ -1126,8 +1134,6 @@ private:
///Stores the environment-information for the given url
void storeInformationList(const IndexedString& url)
{
QMutexLocker lock(m_environmentListInfo.mutex());
EnvironmentInformationListItem newItem;
newItem.m_file = url;
......@@ -1147,6 +1153,7 @@ private:
}
}
QMutexLocker lock(&m_environmentListInfoMutex);
uint index = m_environmentListInfo.findIndex(url);
if (index) {
......@@ -1185,11 +1192,12 @@ private:
///The following repositories are thread-safe, and m_chainsMutex should not be locked when using them, because
///they may trigger I/O. Still it may be required to lock their local mutexes.
///Maps filenames to a list of top-contexts/environment-information.
QMutex m_environmentListInfoMutex = QMutex(QMutex::Recursive);
ItemRepository<EnvironmentInformationListItem, EnvironmentInformationListRequest> m_environmentListInfo;
QMutex m_environmentListInfoMutex;
ItemRepository<EnvironmentInformationListItem, EnvironmentInformationListRequest, true, false>
m_environmentListInfo;
///Maps top-context-indices to environment-information item.
QMutex m_environmentInfoMutex = QMutex(QMutex::Recursive);
ItemRepository<EnvironmentInformationItem, EnvironmentInformationRequest> m_environmentInfo;
QMutex m_environmentInfoMutex;
ItemRepository<EnvironmentInformationItem, EnvironmentInformationRequest, true, false> m_environmentInfo;
};
Q_GLOBAL_STATIC(DUChainPrivate, sdDUChainPrivate)
......
......@@ -2336,6 +2336,16 @@ public:
return m_d.findIndex(request);
}
const Item* findItem(const ItemRequest& request)
{
ThisLocker lock(m_mutex);
auto index = m_d.findIndex(request);
if (!index) {
return nullptr;
}
return m_d.itemFromIndex(index);
}
/// Deletes the item from the repository.
void deleteItem(unsigned int index)
{
......
Supports Markdown
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