SVN commit 891018 by zwabel: Make the destructor/constructor calls in the duchain data classes "persistent". This means that whenever a duchain item data is copied, the copy-constructor is called, and whenever a duchain item data is deleted the destructor is called. The important trick: The destructor is _not_ called when the duchain item has been stored to disk. Then, only the freeDynamicData() function is called, which releases used memory. This allows doing easy and convenient persistent C++-like reference-counting in the data classes, relying purely on the constructors and destructors. M +4 -2 appendedlist.h M +13 -8 declaration.cpp M +24 -14 duchain.cpp M +7 -7 duchainbase.cpp M +16 -0 duchainbase.h M +7 -0 duchainregister.cpp M +11 -0 duchainregister.h M +2 -0 ducontext.cpp M +4 -4 topducontext.cpp M +9 -1 topducontextdynamicdata.cpp M +5 -0 topducontextdynamicdata.h M +3 -0 types/typesystemdata.h --- trunk/KDE/kdevplatform/language/duchain/appendedlist.h #891017:891018 @@ -200,13 +200,15 @@ ///use this if the class does not have a base class that also uses appended lists #define START_APPENDED_LISTS(container) \ -unsigned int offsetBehindBase() const { return 0; } +unsigned int offsetBehindBase() const { return 0; } \ +void freeDynamicData() { freeAppendedLists(); } ///Use this if one of the base-classes of the container also has the appended lists interfaces implemented. ///To reduce the probability of future problems, you should give the direct base class this one inherits from. ///@note: Multiple inheritance is not supported, however it will work ok if only one of the base-classes uses appended lists. #define START_APPENDED_LISTS_BASE(container, base) \ -unsigned int offsetBehindBase() const { return base :: offsetBehindLastList(); } +unsigned int offsetBehindBase() const { return base :: offsetBehindLastList(); } \ +void freeDynamicData() { freeAppendedLists(); base::freeDynamicData(); } #define APPENDED_LIST_COMMON(container, type, name) \ --- trunk/KDE/kdevplatform/language/duchain/declaration.cpp #891017:891018 @@ -140,8 +140,12 @@ Declaration::~Declaration() { + uint oldOwnIndex = m_indexInTopContext; + + TopDUContext* topContext = this->topContext(); + //Only perform the actions when the top-context isn't being deleted, or when it hasn't been stored to disk - if(!topContext()->deleting() || !topContext()->isOnDisk()) { + if(!topContext->deleting() || !topContext->isOnDisk()) { DUCHAIN_D_DYNAMIC(Declaration); // Inserted by the builder after construction has finished. if( d->m_internalContext.context() ) @@ -156,20 +160,21 @@ d->m_inSymbolTable = false; } - // If the parent-context already has dynamic data, like for example any temporary context, - // always delete the declaration, to not create crashes within more complex code like C++ template stuff. - if (context() && !d_func()->m_anonymousInContext) { - if(!topContext()->deleting() || !topContext()->isOnDisk() || context()->d_func()->isDynamic()) - Q_ASSERT(context()->m_dynamicData->removeDeclaration(this)); - } + // If the parent-context already has dynamic data, like for example any temporary context, + // always delete the declaration, to not create crashes within more complex code like C++ template stuff. + if (context() && !d_func()->m_anonymousInContext) { + if(!topContext->deleting() || !topContext->isOnDisk() || context()->d_func()->isDynamic()) + Q_ASSERT(context()->m_dynamicData->removeDeclaration(this)); + } clearOwnIndex(); - if(!topContext()->deleting() || !topContext()->isOnDisk()) { + if(!topContext->deleting() || !topContext->isOnDisk()) { setContext(0); setAbstractType(AbstractType::Ptr()); } + Q_ASSERT(d_func()->isDynamic() == (!topContext->deleting() || !topContext->isOnDisk() || topContext->m_dynamicData->isTemporaryDeclarationIndex(oldOwnIndex))); //DUChain::declarationChanged(this, DUChainObserver::Deletion, DUChainObserver::NotApplicable); } --- trunk/KDE/kdevplatform/language/duchain/duchain.cpp #891017:891018 @@ -127,7 +127,7 @@ size_t itemSize() const { return sizeof(EnvironmentInformationItem) + DUChainItemSystem::self().dynamicSize(*m_file->d_func()); } - + void createItem(EnvironmentInformationItem* item) const { new (item) EnvironmentInformationItem(m_index, DUChainItemSystem::self().dynamicSize(*m_file->d_func())); @@ -340,13 +340,16 @@ Q_ASSERT(info->d_func()->classId); } - ///The item must managed currently + ///The item must be managed currently void removeEnvironmentInformation(ParsingEnvironmentFilePointer info) { + info->makeDynamic(); //By doing this, we make sure the data is actually being destroyed in the destructor + bool removed = (bool)m_fileEnvironmentInformations.remove(info->url(), info); uint index = m_environmentInfo.findIndex(info->indexedTopContext().index()); - if(index) + if(index) { m_environmentInfo.deleteItem(index); + } Q_ASSERT(index || removed); } @@ -611,16 +614,20 @@ } - for(QMultiMap::iterator it = m_fileEnvironmentInformations.begin(); it != m_fileEnvironmentInformations.end(); ) { - ParsingEnvironmentFile* f = (*it).data(); - Q_ASSERT(f->d_func()->classId); - if(f->ref == 1) { - //The ParsingEnvironmentFilePointer is only referenced once. This means that it's does not belong to any - //loaded top-context, so just remove it to save some memory and processing time. - ///@todo use some kind of timeout before removing - it = m_fileEnvironmentInformations.erase(it); - }else{ - ++it; + if(retries == 0) { + //Do this atomically, since we must be sure that _everything_ is already saved + for(QMultiMap::iterator it = m_fileEnvironmentInformations.begin(); it != m_fileEnvironmentInformations.end(); ) { + ParsingEnvironmentFile* f = (*it).data(); + Q_ASSERT(f->d_func()->classId); + if(f->ref == 1) { + Q_ASSERT(!f->d_func()->isDynamic()); //It cannot be dynamic, since we have stored before + //The ParsingEnvironmentFilePointer is only referenced once. This means that it does not belong to any + //loaded top-context, so just remove it to save some memory and processing time. + ///@todo use some kind of timeout before removing + it = m_fileEnvironmentInformations.erase(it); + }else{ + ++it; + } } } @@ -825,8 +832,11 @@ branchRemoved(context); - if(!context->isOnDisk()) + if(!context->isOnDisk()) { removeFromEnvironmentManager(context); + }else{ + context->m_dynamicData->store(); + } context->deleteSelf(); --- trunk/KDE/kdevplatform/language/duchain/duchainbase.cpp #891017:891018 @@ -70,19 +70,15 @@ void DUChainBase::setData(DocumentRangeObjectData* data) { - if(d_func()->m_dynamic) - //We only delete the data when it's dynamic, because else it is embedded in an array in the top-context. - KDevelop::DUChainItemSystem::self().callDestructor(d_func_dynamic()); + KDevelop::DUChainItemSystem::self().callDestructor(static_cast(d_ptr)); DocumentRangeObject::setData(data); } DUChainBase::~DUChainBase() { - if(d_func()->m_dynamic) { - //We only delete the data when it's dynamic, because else it is embedded in an array in the top-context. + if(d_func()->m_dynamic) KDevelop::DUChainItemSystem::self().callDestructor(d_func_dynamic()); - } if (m_ptr) m_ptr->m_base = 0; @@ -115,8 +111,11 @@ Q_ASSERT(d_ptr); if(!d_func()->m_dynamic) { Q_ASSERT(d_func()->classId); + DUChainBaseData* newData = DUChainItemSystem::self().cloneData(*d_func()); //We don't delete the previous data, because it's embedded in the top-context when it isn't dynamic. - d_ptr = DUChainItemSystem::self().cloneData(*d_func()); + //However we do call the destructor, to keep semantic stuff like reference-counting within the data class working correctly. + KDevelop::DUChainItemSystem::self().callDestructor(static_cast(d_ptr)); + d_ptr = newData; Q_ASSERT(d_ptr); Q_ASSERT(d_func()->m_dynamic); Q_ASSERT(d_func()->classId); @@ -137,6 +136,7 @@ else shouldCreateConstantDataStorage.setLocalData(0); } + } // kate: space-indent on; indent-width 2; tab-width 4; replace-tabs on; auto-insert-doxygen on --- trunk/KDE/kdevplatform/language/duchain/duchainbase.h #891017:891018 @@ -43,6 +43,15 @@ #define DUCHAIN_D(Class) const Class##Data * const d = d_func() #define DUCHAIN_D_DYNAMIC(Class) Class##Data * const d = d_func_dynamic() +///@note When a data-item is stored on disk, no destructors of contained items will be called while destruction. +///DUChainBase assumes that each item that has constant data, is stored on disk. +///However the destructor is called even on constant items, when they have been replaced with a dynamic item. +///This tries to keep constructor/destructor count consistency persistently, which allows doing static reference-counting +///using contained classes in their constructor/destructors(For example the class Utils::StorableSet). +///This means that the data of all items that are stored to disk _MUST_ be made constant before their destruction. +///This also means that every item that is "semantically" deleted, _MUST_ have dynamic data before its destruction. +///This also means that DUChainBaseData based items should never be cloned using memcpy, but rather always using the copy-constructor, +///even if both sides are constant. struct KDEVPLATFORMLANGUAGE_EXPORT DUChainBaseData : public DocumentRangeObjectData { DUChainBaseData() : classId(0) { } @@ -64,6 +73,11 @@ uint classSize() const; + ///This is called whenever the data-object is being deleted memory-wise, but not semantically(Which means it stays on disk) + ///Implementations of parent-classes must always be called + void freeDynamicData() { + } + ///Used to decide whether a constructed item should create constant data. ///The default is "false", so dynamic data is created by default. ///This is stored thread-locally. @@ -116,6 +130,8 @@ DUChainBase( DUChainBaseData& dd ); + ///This must only be used to change the storage-location or storage-kind(dynamic/constant) of the data, but + ///the data must always be equal! virtual void setData(DocumentRangeObjectData*); protected: --- trunk/KDE/kdevplatform/language/duchain/duchainregister.cpp #891017:891018 @@ -40,6 +40,13 @@ return m_factories[data->classId]->callDestructor(data); } +void DUChainItemSystem::freeDynamicData(KDevelop::DUChainBaseData* data) const { + if(uint(m_factories.size()) <= data->classId || m_factories[data->classId] == 0) + return; + return m_factories[data->classId]->freeDynamicData(data); + +} + uint DUChainItemSystem::dynamicSize(const DUChainBaseData& data) const { if(uint(m_factories.size()) <= data.classId || m_factories[data.classId] == 0) return 0; --- trunk/KDE/kdevplatform/language/duchain/duchainregister.h #891017:891018 @@ -32,6 +32,7 @@ public: virtual DUChainBase* create(DUChainBaseData* data) const = 0; virtual void callDestructor(DUChainBaseData* data) const = 0; + virtual void freeDynamicData(DUChainBaseData* data) const = 0; virtual void copy(const DUChainBaseData& from, DUChainBaseData& to, bool constant) const = 0; virtual DUChainBaseData* cloneData(const DUChainBaseData& data) const = 0; virtual uint dynamicSize(const DUChainBaseData& data) const = 0; @@ -64,6 +65,11 @@ static_cast(data)->~Data(); } + void freeDynamicData(DUChainBaseData* data) const { + Q_ASSERT(data->classId == T::Identity); + static_cast(data)->freeDynamicData(); + } + uint dynamicSize(const DUChainBaseData& data) const { Q_ASSERT(data.classId == T::Identity); return static_cast(data).dynamicSize(); @@ -133,8 +139,13 @@ size_t dataClassSize(const DUChainBaseData& data) const; ///Calls the destructor, but does not delete anything. This is needed because the data classes must not contain virtual members. + ///This should only be called when a duchain data-pointer is semantically deleted, eg. when it does not persist on disk. void callDestructor(DUChainBaseData* data) const; + ///Does not call the destructor, but frees all special data associated to dynamic data(the appendedlists stuff) + ///This needs to be called whenever a dynamic duchain data-pointer is being deleted. + void freeDynamicData(DUChainBaseData* data) const; + /// Access the static DUChainItemSystem instance. static DUChainItemSystem& self(); --- trunk/KDE/kdevplatform/language/duchain/ducontext.cpp #891017:891018 @@ -586,6 +586,8 @@ top->m_dynamicData->clearContextIndex(this); } + + Q_ASSERT(d_func()->isDynamic() == (!top->deleting() || !top->isOnDisk() || top->m_dynamicData->isTemporaryContextIndex(m_dynamicData->m_indexInTopContext))); } QVector< DUContext * > DUContext::childContexts( ) const --- trunk/KDE/kdevplatform/language/duchain/topducontext.cpp #891017:891018 @@ -802,7 +802,6 @@ DUCHAIN_D_DYNAMIC(TopDUContext); d->m_features = VisibleDeclarationsAndContexts; - d->m_deleting = false; d->m_ownIndex = m_local->m_ownIndex; m_local->m_file = ParsingEnvironmentFilePointer(file); setInSymbolTable(true); @@ -823,7 +822,7 @@ TopDUContext::~TopDUContext( ) { if(!m_local->m_sharedDataOwner) { - d_func_dynamic()->m_deleting = true; + m_dynamicData->m_deleting = true; if(!isOnDisk()) clearUsedDeclarationIndices(); } @@ -835,7 +834,7 @@ TopDUContextDynamicData* dynamicData = m_dynamicData; if(!m_local->m_sharedDataOwner) - d_func_dynamic()->m_deleting = true; + m_dynamicData->m_deleting = true; delete this; @@ -1225,7 +1224,8 @@ bool TopDUContext::deleting() const { - return d_func()->m_deleting; + ///@todo remove d_func()->m_deleting, not used any more + return m_dynamicData->m_deleting; } QList TopDUContext::problems() const --- trunk/KDE/kdevplatform/language/duchain/topducontextdynamicdata.cpp #891017:891018 @@ -59,7 +59,7 @@ item.setData(&target); } -TopDUContextDynamicData::TopDUContextDynamicData(TopDUContext* topContext) : m_topContext(topContext), m_fastContexts(0), m_fastContextsSize(0), m_fastDeclarations(0), m_fastDeclarationsSize(0), m_onDisk(false), m_dataLoaded(true) { +TopDUContextDynamicData::TopDUContextDynamicData(TopDUContext* topContext) : m_topContext(topContext), m_fastContexts(0), m_fastContextsSize(0), m_fastDeclarations(0), m_fastDeclarationsSize(0), m_onDisk(false), m_dataLoaded(true), m_deleting(false) { } TopDUContextDynamicData::~TopDUContextDynamicData() { @@ -473,6 +473,14 @@ } } +bool TopDUContextDynamicData::isTemporaryContextIndex(uint index) const { + return !(index < (0x0fffffff/2)); +} + +bool TopDUContextDynamicData::isTemporaryDeclarationIndex(uint index) const { + return !(index < (0x0fffffff/2)); +} + DUContext* TopDUContextDynamicData::getContextForIndex(uint index) const { if(!m_dataLoaded) --- trunk/KDE/kdevplatform/language/duchain/topducontextdynamicdata.h #891017:891018 @@ -86,6 +86,11 @@ static QList loadImports(uint topContextIndex); + bool isTemporaryContextIndex(uint index) const; + bool isTemporaryDeclarationIndex(uint index) const ; + + bool m_deleting; ///Flag used during destruction + private: void loadData() const; --- trunk/KDE/kdevplatform/language/duchain/types/typesystemdata.h #891017:891018 @@ -85,6 +85,9 @@ /// Expensive unsigned int hash() const; + void freeDynamicData() { + } + private: AbstractTypeData& operator=(const AbstractTypeData&); };