Commit 9ff261fa authored by Milian Wolff's avatar Milian Wolff
Browse files

Always assume external mutex locking for ItemRepository

Instead, take the mutex type which allows us to use either a
QRecursiveMutex or a normal QMutex. Then simplify the code
and remove the ItemRepositoryPrivate again, which is now obsolete.
parent 9d3d0653
......@@ -167,7 +167,7 @@ class CodeModelPrivate
public:
mutable QMutex m_mutex;
//Maps declaration-ids to items
using Repo = ItemRepository<CodeModelRepositoryItem, CodeModelRequestItem, true, false>;
using Repo = ItemRepository<CodeModelRepositoryItem, CodeModelRequestItem>;
// mutable as things like findIndex are not const
mutable Repo m_repository{QStringLiteral("Code Model"), &m_mutex};
};
......
......@@ -151,7 +151,7 @@ class DefinitionsPrivate
public:
mutable QMutex m_mutex;
//Maps declaration-ids to definitions
using Repo = ItemRepository<DefinitionsItem, DefinitionsRequestItem, true, false>;
using Repo = ItemRepository<DefinitionsItem, DefinitionsRequestItem>;
// mutable as things like findIndex are not const
mutable Repo m_definitions{QStringLiteral("Definition Map"), &m_mutex};
};
......
......@@ -1193,11 +1193,10 @@ private:
///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;
ItemRepository<EnvironmentInformationListItem, EnvironmentInformationListRequest, true, false>
m_environmentListInfo;
ItemRepository<EnvironmentInformationListItem, EnvironmentInformationListRequest> m_environmentListInfo;
///Maps top-context-indices to environment-information item.
QMutex m_environmentInfoMutex;
ItemRepository<EnvironmentInformationItem, EnvironmentInformationRequest, true, false> m_environmentInfo;
ItemRepository<EnvironmentInformationItem, EnvironmentInformationRequest> m_environmentInfo;
};
Q_GLOBAL_STATIC(DUChainPrivate, sdDUChainPrivate)
......
......@@ -147,14 +147,14 @@ struct IdentifierItemRequest
const DynamicIdentifierPrivate& m_identifier;
};
using IdentifierRepository = ItemRepository<ConstantIdentifierPrivate, IdentifierItemRequest, true, false>;
using IdentifierRepository = ItemRepository<ConstantIdentifierPrivate, IdentifierItemRequest, true, QRecursiveMutex>;
using IdentifierRepositoryManager = RepositoryManager<IdentifierRepository, false>;
template <>
struct ItemRepositoryForItemType<IndexedIdentifier> {
static IdentifierRepository& repo()
{
static QMutex mutex(QMutex::Recursive);
static QRecursiveMutex mutex;
static IdentifierRepositoryManager manager(QStringLiteral("Identifier Repository"), &mutex);
return *manager.repository();
}
......@@ -325,7 +325,7 @@ struct QualifiedIdentifierItemRequest
};
using QualifiedIdentifierRepository
= ItemRepository<ConstantQualifiedIdentifierPrivate, QualifiedIdentifierItemRequest, true, false>;
= ItemRepository<ConstantQualifiedIdentifierPrivate, QualifiedIdentifierItemRequest, true, QRecursiveMutex>;
using QualifiedIdentifierRepositoryManager = RepositoryManager<QualifiedIdentifierRepository, false>;
template <>
......
......@@ -107,7 +107,7 @@ class ImportersPrivate
public:
mutable QMutex m_mutex;
//Maps declaration-ids to Importers
using Repo = ItemRepository<ImportersItem, ImportersRequestItem, true, false>;
using Repo = ItemRepository<ImportersItem, ImportersRequestItem>;
// mutable as things like findIndex are not const
mutable Repo m_importers{QStringLiteral("Importer Map"), &m_mutex};
};
......
......@@ -118,7 +118,8 @@ uint InstantiationInformation::hash() const
}
using InstantiationInformationRepository
= ItemRepository<InstantiationInformation, AppendedListItemRequest<InstantiationInformation>, true, false>;
= ItemRepository<InstantiationInformation, AppendedListItemRequest<InstantiationInformation>, true,
QRecursiveMutex>;
using InstantiationInformationRepositoryManager = RepositoryManager<InstantiationInformationRepository>;
template <>
......
......@@ -42,7 +42,7 @@ constexpr TextStreamFunction endl = ::endl;
Utils::BasicSetRepository* RecursiveImportCacheRepository::repository()
{
static auto mutex = QMutex(QMutex::Recursive);
static QRecursiveMutex mutex;
static Utils::BasicSetRepository recursiveImportCacheRepositoryObject(QStringLiteral("Recursive Imports Cache"),
&mutex, nullptr, false);
return &recursiveImportCacheRepositoryObject;
......@@ -154,7 +154,7 @@ class PersistentSymbolTablePrivate
public:
QMutex m_mutex;
//Maps declaration-ids to declarations
using Repo = ItemRepository<PersistentSymbolTableItem, PersistentSymbolTableRequestItem, true, false>;
using Repo = ItemRepository<PersistentSymbolTableItem, PersistentSymbolTableRequestItem>;
// mutable as things like findIndex are not const
mutable Repo m_declarations{QStringLiteral("Persistent Declaration Table"), &m_mutex};
......
......@@ -88,7 +88,7 @@ void TestDUChain::testStringSets()
const unsigned int choiceCount = 40;
const unsigned int itemCount = 120;
auto mutex = QMutex(QMutex::Recursive);
QRecursiveMutex mutex;
BasicSetRepository rep(QStringLiteral("test repository"), &mutex);
// qDebug() << "Start repository-layout: \n" << rep.dumpDotGraph();
......
......@@ -38,7 +38,7 @@ const uint maxApplyAliasesRecursion = 100;
namespace KDevelop {
Utils::BasicSetRepository* RecursiveImportRepository::repository()
{
static auto mutex = QMutex(QMutex::Recursive);
static QRecursiveMutex mutex;
static Utils::BasicSetRepository recursiveImportRepositoryObject(QStringLiteral("Recursive Imports"), &mutex,
&KDevelop::globalItemRepositoryRegistry());
return &recursiveImportRepositoryObject;
......
......@@ -84,13 +84,13 @@ public:
const AbstractType& m_item;
};
QMutex* typeRepositoryMutex()
QRecursiveMutex* typeRepositoryMutex()
{
static auto mutex = QMutex(QMutex::Recursive);
static QRecursiveMutex mutex;
return &mutex;
}
using TypeItemRepository = ItemRepository<AbstractTypeData, AbstractTypeDataRequest, true, false>;
using TypeItemRepository = ItemRepository<AbstractTypeData, AbstractTypeDataRequest, true, QRecursiveMutex>;
// The object is created in a function, to prevent initialization-order issues
static TypeItemRepository& typeRepository()
......
......@@ -9,7 +9,7 @@
#include <language/duchain/types/abstracttype.h>
class QMutex;
class QRecursiveMutex;
namespace KDevelop {
struct ReferenceCountManager;
......@@ -25,7 +25,7 @@ public:
static void decreaseReferenceCount(uint index, ReferenceCountManager* manager);
};
QMutex* typeRepositoryMutex();
QRecursiveMutex* typeRepositoryMutex();
}
#endif
......@@ -107,7 +107,7 @@ class UsesPrivate
public:
mutable QMutex m_mutex;
//Maps declaration-ids to Uses
using Repo = ItemRepository<UsesItem, UsesRequestItem, true, false>;
using Repo = ItemRepository<UsesItem, UsesRequestItem>;
// mutable as things like findIndex are not const
mutable Repo m_uses{QStringLiteral("Use Map"), &m_mutex};
};
......
......@@ -15,7 +15,11 @@
// #define DEBUG_NEEDSUPDATE
namespace KDevelop {
QMutex modificationRevisionSetMutex(QMutex::Recursive);
QRecursiveMutex* modificationRevisionSetMutex()
{
static QRecursiveMutex mutex;
return &mutex;
}
struct FileModificationPair
{
......@@ -93,12 +97,13 @@ struct FileModificationPairRequest
}
};
using FileModificationPairRepository = KDevelop::ItemRepository<FileModificationPair, FileModificationPairRequest, true, false>;
using FileModificationPairRepository
= KDevelop::ItemRepository<FileModificationPair, FileModificationPairRequest, true, QRecursiveMutex>;
static FileModificationPairRepository& fileModificationPairRepository()
{
static FileModificationPairRepository rep(QStringLiteral("file modification repository"),
&modificationRevisionSetMutex);
modificationRevisionSetMutex());
return rep;
}
......@@ -111,7 +116,7 @@ QHash<uint, std::pair<QDateTime, bool>> needsUpdateCache;
void ModificationRevisionSet::clearCache()
{
QMutexLocker lock(&modificationRevisionSetMutex);
QMutexLocker lock(modificationRevisionSetMutex());
///@todo More intelligent clearing. We actually need to watch the directory for changes, and if there are changes, clear the cache.
needsUpdateCache.clear();
}
......@@ -120,7 +125,7 @@ struct FileModificationSetRepository
: public Utils::BasicSetRepository
{
FileModificationSetRepository()
: Utils::BasicSetRepository(QStringLiteral("file modification sets"), &modificationRevisionSetMutex,
: Utils::BasicSetRepository(QStringLiteral("file modification sets"), modificationRevisionSetMutex(),
&globalItemRepositoryRegistry(), true)
{
}
......@@ -150,7 +155,7 @@ uint ModificationRevisionSet::size() const
void ModificationRevisionSet::clear()
{
QMutexLocker lock(&modificationRevisionSetMutex);
QMutexLocker lock(modificationRevisionSetMutex());
if (m_index) {
Utils::Set oldModificationTimes = Utils::Set(m_index, &FileModificationSetRepositoryRepresenter::repository());
......@@ -162,7 +167,7 @@ void ModificationRevisionSet::clear()
void ModificationRevisionSet::addModificationRevision(const IndexedString& url,
const KDevelop::ModificationRevision& revision)
{
QMutexLocker lock(&modificationRevisionSetMutex);
QMutexLocker lock(modificationRevisionSetMutex());
if (m_index == 0) {
Utils::Set set = FileModificationSetRepositoryRepresenter::repository().createSet(
......@@ -189,7 +194,7 @@ void ModificationRevisionSet::addModificationRevision(const IndexedString& url,
bool ModificationRevisionSet::removeModificationRevision(const IndexedString& url,
const KDevelop::ModificationRevision& revision)
{
QMutexLocker lock(&modificationRevisionSetMutex);
QMutexLocker lock(modificationRevisionSetMutex());
if (!m_index)
return false;
......@@ -231,7 +236,7 @@ using ModificationRevisionSetNode = Utils::VirtualSetNode<uint, Utils::IdentityC
static bool nodeNeedsUpdate(uint index)
{
QMutexLocker lock(&modificationRevisionSetMutex);
QMutexLocker lock(modificationRevisionSetMutex());
if (!index)
return false;
......@@ -269,7 +274,7 @@ static bool nodeNeedsUpdate(uint index)
QString ModificationRevisionSet::toString() const
{
QMutexLocker lock(&modificationRevisionSetMutex);
QMutexLocker lock(modificationRevisionSetMutex());
Utils::Set set(m_index, &FileModificationSetRepositoryRepresenter::repository());
Utils::Set::Iterator it = set.iterator();
QStringList revisions;
......@@ -285,7 +290,7 @@ QString ModificationRevisionSet::toString() const
bool ModificationRevisionSet::needsUpdate() const
{
QMutexLocker lock(&modificationRevisionSetMutex);
QMutexLocker lock(modificationRevisionSetMutex());
#ifdef DEBUG_NEEDSUPDATE
Utils::Set set(m_index, &FileModificationSetRepositoryRepresenter::repository());
......@@ -308,7 +313,7 @@ bool ModificationRevisionSet::needsUpdate() const
ModificationRevisionSet& ModificationRevisionSet::operator+=(const ModificationRevisionSet& rhs)
{
QMutexLocker lock(&modificationRevisionSetMutex);
QMutexLocker lock(modificationRevisionSetMutex());
Utils::Set oldModificationTimes = Utils::Set(m_index, &FileModificationSetRepositoryRepresenter::repository());
Utils::Set otherModificationTimes =
......@@ -327,7 +332,7 @@ ModificationRevisionSet& ModificationRevisionSet::operator+=(const ModificationR
ModificationRevisionSet& ModificationRevisionSet::operator-=(const ModificationRevisionSet& rhs)
{
QMutexLocker lock(&modificationRevisionSetMutex);
QMutexLocker lock(modificationRevisionSetMutex());
Utils::Set oldModificationTimes = Utils::Set(m_index, &FileModificationSetRepositoryRepresenter::repository());
Utils::Set otherModificationTimes =
......
......@@ -120,8 +120,8 @@ public:
}
};
using SetDataRepositoryBase =
KDevelop::ItemRepository<SetNodeData, SetNodeDataRequest, false, false, sizeof(SetNodeData)>;
using SetDataRepositoryBase
= KDevelop::ItemRepository<SetNodeData, SetNodeDataRequest, false, QRecursiveMutex, sizeof(SetNodeData)>;
struct SetDataRepository;
......@@ -179,7 +179,7 @@ struct KDEVPLATFORMLANGUAGE_EXPORT SetDataRepository
: public SetDataRepositoryBase
{
SetDataRepository(BasicSetRepository* _setRepository, const QString& name,
KDevelop::ItemRepositoryRegistry* registry, QMutex* mutex)
KDevelop::ItemRepositoryRegistry* registry, QRecursiveMutex* mutex)
: SetDataRepositoryBase(name, mutex, registry)
, setRepository(_setRepository)
{
......@@ -272,7 +272,7 @@ class KDEVPLATFORMLANGUAGE_EXPORT BasicSetRepository
public:
///@param name The name must be unique, and is used for loading and storing the data
///@param registry Where the repository should be registered. If you give zero, it won't be registered, and thus won't be saved to disk.
explicit BasicSetRepository(const QString& name, QMutex* mutex,
explicit BasicSetRepository(const QString& name, QRecursiveMutex* mutex,
KDevelop::ItemRepositoryRegistry* registry = &KDevelop::globalItemRepositoryRegistry(),
bool delayedDeletion = delayedDeletionByDefault);
virtual ~BasicSetRepository();
......@@ -332,7 +332,7 @@ private:
friend class Set;
friend class Set::Iterator;
SetDataRepository m_dataRepository;
QMutex* m_mutex;
QRecursiveMutex* m_mutex;
bool m_delayedDeletion;
// SetNode
......
......@@ -941,8 +941,8 @@ Set BasicSetRepository::createSet(const std::set<Index>& indices)
return createSetFromIndices(indicesVector);
}
BasicSetRepository::BasicSetRepository(const QString& name, QMutex* mutex, KDevelop::ItemRepositoryRegistry* registry,
bool delayedDeletion)
BasicSetRepository::BasicSetRepository(const QString& name, QRecursiveMutex* mutex,
KDevelop::ItemRepositoryRegistry* registry, bool delayedDeletion)
: m_dataRepository(this, name, registry, mutex)
, m_mutex(nullptr)
, m_delayedDeletion(delayedDeletion)
......@@ -1173,7 +1173,7 @@ void Set::staticUnref()
unrefNode(m_tree);
}
StringSetRepository::StringSetRepository(const QString& name, QMutex* mutex)
StringSetRepository::StringSetRepository(const QString& name, QRecursiveMutex* mutex)
: Utils::BasicSetRepository(name, mutex)
{
}
......
......@@ -532,7 +532,7 @@ private:
struct KDEVPLATFORMLANGUAGE_EXPORT StringSetRepository
: public Utils::BasicSetRepository
{
explicit StringSetRepository(const QString& name, QMutex* mutex);
explicit StringSetRepository(const QString& name, QRecursiveMutex* mutex);
void itemRemovedFromSets(uint index) override;
void itemAddedToSets(uint index) override;
};
......
......@@ -7,10 +7,10 @@
#ifndef ABSTRACTITEMREPOSITORY_H
#define ABSTRACTITEMREPOSITORY_H
#include <QMutex>
#include "serializationexport.h"
#include <QtGlobal>
class QString;
namespace KDevelop {
......@@ -44,8 +44,6 @@ public:
void deleteRepository();
virtual QMutex* repositoryMutex() const = 0;
protected:
mutable AbstractItemRepository* m_repository = nullptr;
};
......
......@@ -132,7 +132,7 @@ inline char indexToChar(uint index)
return static_cast<char>(index & 0xff);
}
using IndexedStringRepository = ItemRepository<IndexedStringData, IndexedStringRepositoryItemRequest, false, false>;
using IndexedStringRepository = ItemRepository<IndexedStringData, IndexedStringRepositoryItemRequest, false>;
using IndexedStringRepositoryManagerBase = RepositoryManager<IndexedStringRepository, true, false>;
class IndexedStringRepositoryManager
: public IndexedStringRepositoryManagerBase
......
......@@ -10,6 +10,8 @@
#include <QDebug>
#include <QDir>
#include <QFile>
#include <QMutex>
#include <QMutexLocker>
#include <KMessageBox>
#include <KLocalizedString>
......@@ -59,6 +61,7 @@
// #define DEBUG_WRITING_EXTENTS
class TestItemRepository;
class BenchItemRepository;
namespace KDevelop {
/**
......@@ -540,7 +543,7 @@ public:
{
const OptionalDUChainReferenceCountingEnabler<markForReferenceCounting> optionalRc(m_data, dataSize());
ItemRequest::destroy(item, repository.itemRepository());
ItemRequest::destroy(item, repository);
}
#ifndef QT_NO_DEBUG
......@@ -986,32 +989,6 @@ private:
mutable int m_lastUsed = 0; //How many ticks ago this bucket was last accessed
};
template <bool lock>
struct Locker //This is a dummy that does nothing
{
template <class T>
explicit Locker(const T& /*t*/)
{
}
};
template <>
struct Locker<true>
{
explicit Locker(QMutex* mutex) : m_mutex(mutex)
{
m_mutex->lock();
}
~Locker()
{
m_mutex->unlock();
}
private:
Q_DISABLE_COPY(Locker)
QMutex* m_mutex;
};
///This object needs to be kept alive as long as you change the contents of an item
///stored in the repository. It is needed to correctly track the reference counting
///within disk-storage.
......@@ -1033,15 +1010,30 @@ private:
};
/**
* The internal implementation of the ItemRepository
* The ItemRepository is essentially an on-disk key/value hash map
*
* Access to this structure is assumed to happen in a thread safe manner, e.g.
* by locking a mutex externally. The private API can then call itself without
* having to re-lock anything internally, thus is capable to work with a non-recursive mutex.
* by locking a mutex externally. The API can then call itself without having
* to re-lock anything internally, thus is capable to work with a non-recursive mutex.
* If desired, a recursive mutex can be used too though as needed.
*
* Note that the mutex will be locked internally when the repo is accessed through the AbstractItemRepository API.
*
* @tparam Item See ExampleItem
* @tparam ItemRequest See ExampleReqestItem
* @tparam fixedItemSize When this is true, all inserted items must have the same size.
* This greatly simplifies and speeds up the task of managing free items within the buckets.
* @tparam markForReferenceCounting Whether the data within the repository should be marked for reference-counting.
* This costs a bit of performance, but must be enabled if there may be data in the
* repository that does on-disk reference counting, like IndexedString,
* IndexedIdentifier, etc.
* @tparam mutex The mutex type to use internally. It has to be locked externally before accessing the item repository
* from multiple threads. Except for the store
*/
template <class ItemRepository, class Item, class ItemRequest, bool markForReferenceCounting, uint fixedItemSize,
unsigned int targetBucketHashSize>
class ItemRepositoryPrivate
template <class Item, class ItemRequest, bool markForReferenceCounting = true, typename Mutex = QMutex,
uint fixedItemSize = 0, unsigned int targetBucketHashSize = 524288 * 2>
class ItemRepository : public AbstractItemRepository
{
using MyBucket = Bucket<Item, ItemRequest, markForReferenceCounting, fixedItemSize>;
......@@ -1055,15 +1047,22 @@ class ItemRepositoryPrivate
BucketStartOffset = sizeof(uint) * 7 + sizeof(short unsigned int) * bucketHashSize //Position in the data where the bucket array starts
};
Q_DISABLE_COPY(ItemRepositoryPrivate)
Q_DISABLE_COPY_MOVE(ItemRepository)
public:
explicit ItemRepositoryPrivate(ItemRepository& itemRepository, const QString& repositoryName,
uint repositoryVersion = 1)
///@param registry May be zero, then the repository will not be registered at all. Else, the repository will
/// register itself to that registry.
/// If this is zero, you have to care about storing the data using store() and/or close() by
/// yourself. It does not happen automatically. For the global standard registry, the storing/loading
/// is triggered from within duchain, so you don't need to care about it.
explicit ItemRepository(const QString& repositoryName, Mutex* mutex,
ItemRepositoryRegistry* registry = &globalItemRepositoryRegistry(),
uint repositoryVersion = 1, AbstractRepositoryManager* manager = nullptr)
: m_repositoryName(repositoryName)
, m_file(nullptr)
, m_dynamicFile(nullptr)
, m_repositoryVersion(repositoryVersion)
, m_itemRepository(itemRepository)
, m_mutex(mutex)
, m_registry(registry)
{
m_unloadingEnabled = true;
m_metaDataChanged = true;
......@@ -1074,11 +1073,18 @@ public:
m_statBucketHashClashes = m_statItemCount = 0;
m_currentBucket = 1; //Skip the first bucket, we won't use it so we have the zero indices for special purposes
if (m_registry)
m_registry->registerRepository(this, manager);
}
~ItemRepositoryPrivate() { close(); }
~ItemRepository() override
{
if (m_registry)
m_registry->unRegisterRepository(this);
ItemRepository& itemRepository() { return m_itemRepository; }
close();
}
///Unloading of buckets is enabled by default. Use this to disable it. When unloading is enabled, the data
///gotten from must only itemFromIndex must not be used for a long time.
......@@ -1351,7 +1357,7 @@ public:
}
///Returns zero if the item is not in the repository yet
unsigned int findIndex(const ItemRequest& request)
unsigned int findIndex(const ItemRequest& request) const
{
return walkBucketChain(request.hash(), [this, &request](ushort bucketIdx, const MyBucket* bucketPtr) {
const ushort indexInBucket = bucketPtr->findIndex(request);
......@@ -1359,6 +1365,16 @@ public:
});
}
/// Returns nullptr if the item is not in the repository yet
const Item* findItem(const ItemRequest& request) const
{
auto index = findIndex(request);
if (!index) {
return nullptr;
}
return itemFromIndex(index);
}
///Deletes the item from the repository.
void deleteItem(unsigned int index)
{
......@@ -1560,7 +1576,12 @@ public:
}
};
QString printStatistics() const { return statistics().print(); }
QString printStatistics() const override
{
// lock for usage from ItemRepositoryRegistry, will be cleaned up in follow up patch
QMutexLocker lock(m_mutex);
return statistics().print();
}
Statistics statistics() const
{
......@@ -1692,10 +1713,16 @@ public:
}
}
QString repositoryName() const override { return m_repositoryName; }
Mutex* mutex() const { return m_mutex; }
private:
///Synchronizes the state on disk to the one in memory, and does some memory-management.
///Should be called on a regular basis. Can be called centrally from the global item repository registry.
void store()
void store() override
{
QMutexLocker lock(m_mutex);
if (m_file) {
if (!m_file->open(QFile::ReadWrite) || !m_dynamicFile->open(QFile::ReadWrite)) {
qFatal("cannot re-open repository file for storing");
......@@ -1750,11 +1777,10 @@ public:
}
}
QString repositoryName() const { return m_repositoryName; }
bool open(const QString& path)
bool open(const QString& path) override
{
close();
QMutexLocker lock(m_mutex);
closeLocked();
// qDebug() << "opening repository" << m_repositoryName << "at" << path;
QDir dir(path);
m_file = new QFile(dir.absoluteFilePath(m_repositoryName));
......@@ -1869,7 +1895,14 @@ public:
}
///@warning by default, this does not store the current state to disk.
void close(bool doStore = false)