Hello Aron,
On 02/08/2011 02:02 PM, Aron Bierbaum wrote:
I have been testing with the attached changes, which are based of
Carsten's previous patch. I have been testing most of the day and have
not seen any stability issues. The attached patch updates all places
that I found except one,
thanks for finding these, updated patch attached; I'm not totally happy
with exposing the factory's store lock, but I don't see another way to
make iterating over all containers safe otherwise and it should only be
done in a few places anyway.
ChangeList::fillFromCurrentState(). I didn't
know enough about where and how this was used within OpenSG to make the
correct changes.
i adjusted this one as well.
Cheers,
Carsten
diff --git a/Source/Base/FieldContainer/Base/OSGChangeList.cpp
b/Source/Base/FieldContainer/Base/OSGChangeList.cpp
index c7cc6f4..10d05bd 100644
--- a/Source/Base/FieldContainer/Base/OSGChangeList.cpp
+++ b/Source/Base/FieldContainer/Base/OSGChangeList.cpp
@@ -761,44 +761,46 @@ void ChangeList::fillFromCurrentState(UInt32
uiFieldContainerId,
{
this->clear();
- UInt32 uiNumContainers =
- FieldContainerFactory::the()->getNumContainers();
+ FieldContainerFactory::the()->lockStore();
- if(uiNumContainers <= uiFieldContainerId)
- {
- return;
- }
+ FieldContainerFactoryBase::ContainerStoreConstIt sIt =
+ FieldContainerFactory::the()->beginStore();
+ FieldContainerFactoryBase::ContainerStoreConstIt sEnd =
+ FieldContainerFactory::the()->endStore();
- for(UInt32 i = uiFieldContainerId; i < uiNumContainers; ++i)
+ for(; sIt != sEnd; ++sIt)
{
- FieldContainer *pContainer =
- FieldContainerFactory::the()->getContainer(i);
+ if((*sIt).first < uiFieldContainerId)
+ continue;
- // skip destroyed FC
- if(pContainer == NULL)
- continue;
+ FieldContainer *pFC = (*sIt).second->getPtr();
+
+ if(pFC == NULL)
+ continue;
// skip prototypes - unless requested
if(skipPrototypes == true &&
- (pContainer->getType().getPrototype() == pContainer ||
- pContainer->getType().getPrototype() == NULL ))
+ (pFC->getType().getPrototype() == pFC ||
+ pFC->getType().getPrototype() == NULL ))
{
continue;
}
- this->addCreated(i, TypeTraits<BitVector>::BitsClear);
+ this->addCreated((*sIt).first, TypeTraits<BitVector>::BitsClear);
- for(Int32 j = 0; j < pContainer->getRefCount(); ++j)
- this->addAddRefd(i);
+ for(Int32 j = 0; j < pFC->getRefCount(); ++j)
+ this->addAddRefd((*sIt).first);
ContainerChangeEntry *pEntry = this->getNewEntry();
pEntry->uiEntryDesc = ContainerChangeEntry::Change;
- pEntry->pFieldFlags = pContainer->getFieldFlags();
- pEntry->uiContainerId = i;
+ pEntry->pFieldFlags = pFC->getFieldFlags();
+ pEntry->uiContainerId = (*sIt).first;
pEntry->whichField = FieldBits::AllFields;
pEntry->pList = this;
}
+
+ FieldContainerFactory::the()->unlockStore();
}
#ifdef OSG_THREAD_DEBUG_SETASPECTTO
diff --git a/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.cpp
b/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.cpp
index f67b893..82ac842 100644
--- a/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.cpp
+++ b/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.cpp
@@ -78,10 +78,12 @@ for OSG::FieldContainerFactory.
FieldContainerFactoryBase::FieldContainerFactoryBase(void) :
Inherited ("FieldContainerFactory"),
_pStoreLock (NULL ),
- _vContainerStore( ),
+ _nextContainerId(0 ),
+ _containerStore ( ),
_pMapper (NULL )
{
- _vContainerStore.push_back(NULL);
+ _containerStore.insert(
+ ContainerStore::value_type(_nextContainerId++, NULL));
}
FieldContainerFactoryBase::FieldContainerFactoryBase(
@@ -89,10 +91,12 @@ FieldContainerFactoryBase::FieldContainerFactoryBase(
Inherited (szFactoryName),
_pStoreLock (NULL ),
- _vContainerStore( ),
+ _nextContainerId(0 ),
+ _containerStore ( ),
_pMapper (NULL )
{
- _vContainerStore.push_back(NULL);
+ _containerStore.insert(
+ ContainerStore::value_type(_nextContainerId++, NULL));
}
/*-------------------------------------------------------------------------*/
@@ -110,8 +114,8 @@ bool FieldContainerFactoryBase::initialize(void)
if(this->_bInitialized == true)
return true;
- _pStoreLock = ThreadManager::the()->getLock("ContainerFactory::slock",
- false);
+ _pStoreLock = ThreadManager::the()->getLock(
+ "ContainerFactory::_pStorelock", false);
PINFO << "Got store lock " << _pStoreLock.get() << std::endl;
@@ -134,32 +138,33 @@ bool FieldContainerFactoryBase::terminate(void)
this->_bInitialized = false;
#ifdef OSG_DEBUG
- ContainerStoreIt sI = _vContainerStore.begin();
- ContainerStoreIt sE = _vContainerStore.end ();
+ ContainerStoreConstIt sI = _containerStore.begin();
+ ContainerStoreConstIt sE = _containerStore.end ();
- for(UInt32 i = 0; sI != sE; ++sI, ++i)
+ for(; sI != sE; ++sI)
{
- if((*sI) != NULL)
+ if((*sI).second != NULL)
{
FWARNING(("FieldContainerFactoryBase::terminate: "
- "Entry [%d] is not NULL ([%p]). \n", i, *sI));
+ "Entry [%d] is not NULL ([%p]). \n",
+ (*sI).first, (*sI).second));
- for(UInt32 j = 0; j < (*sI)->getNumAspects(); ++j)
+ for(UInt32 j = 0; j < (*sI).second->getNumAspects(); ++j)
{
- if((*sI)->getPtr(j) != NULL)
+ if((*sI).second->getPtr(j) != NULL)
{
FWARNING((" [%d] [%p] [%s] [%d %d]\n",
j,
- (*sI)->getPtr(j),
- (*sI)->getPtr(j)->getType().getCName(),
- (*sI)->getPtr(j)->getRefCount(),
- (*sI)->getPtr(j)->getWeakRefCount() ));
+ (*sI).second->getPtr(j),
+ (*sI).second->getPtr(j)->getType().getCName(),
+ (*sI).second->getPtr(j)->getRefCount(),
+ (*sI).second->getPtr(j)->getWeakRefCount() ));
}
else
{
FWARNING((" [%d] [%p] [] [N/A N/A]\n",
j,
- (*sI)->getPtr(j)));
+ (*sI).second->getPtr(j)));
}
}
}
@@ -182,22 +187,51 @@ UInt32 FieldContainerFactoryBase::registerContainer(
VALGRIND_CHECK_VALUE_IS_DEFINED(pContainer);
#endif
- UInt32 returnValue = 0;
-
_pStoreLock->acquire();
+ UInt32 returnValue = _nextContainerId++;
+
#ifdef OSG_MT_CPTR_ASPECT
ContainerHandlerP pHandler = NULL;
if(pContainer != NULL)
pHandler = pContainer->getAspectStore();
- _vContainerStore.push_back(pHandler);
+ _containerStore.insert(
+ ContainerStore::value_type(returnValue, pHandler));
#else
- _vContainerStore.push_back(pContainer);
+ _containerStore.insert(
+ ContainerStore::value_type(returnValue, pContainer));
#endif
- returnValue = _vContainerStore.size() - 1;
+ _pStoreLock->release();
+
+ return returnValue;
+}
+
+bool FieldContainerFactoryBase::deregisterContainer(const UInt32 uiContainerId)
+{
+ bool returnValue = false;
+
+ _pStoreLock->acquire();
+
+ ContainerStoreIt sI = _containerStore.find(uiContainerId);
+
+ if(sI != _containerStore.end())
+ {
+ _containerStore.erase(sI);
+ }
+#ifdef OSG_DEBUG
+ else
+ {
+ SWARNING << "FieldContainerFactory::unregisterContainer: "
+ << "id '" << uiContainerId << "' not found in "
+ << "container store!"
+ << std::endl;
+
+ returnValue = true;
+ }
+#endif
_pStoreLock->release();
@@ -220,20 +254,19 @@ Int32
FieldContainerFactoryBase::findContainer(ContainerPtr ptr) const
{
_pStoreLock->acquire();
- ContainerStore::const_iterator it, end;
- Int32 id = 0;
- Int32 returnValue = -1;
+ ContainerStoreConstIt sI = _containerStore.begin();
+ ContainerStoreConstIt sE = _containerStore.end ();
+ Int32 returnValue = -1;
- for(it = _vContainerStore.begin(), end = _vContainerStore.end();
- it != end; ++it, ++id)
+ for(; sI != sE; ++sI)
{
#ifdef OSG_MT_CPTR_ASPECT
- if((*it)->getPtr() == ptr)
+ if((*sI).second->getPtr() == ptr)
#else
- if(*it == ptr)
+ if((*sI).second == ptr)
#endif
{
- returnValue = id;
+ returnValue = (*sI).first;
break;
}
}
@@ -250,32 +283,33 @@ void FieldContainerFactoryBase::dump(void)
{
_pStoreLock->acquire();
- ContainerStoreIt sI = _vContainerStore.begin();
- ContainerStoreIt sE = _vContainerStore.end ();
+ ContainerStoreConstIt sI = _containerStore.begin();
+ ContainerStoreConstIt sE = _containerStore.end ();
- for(UInt32 i = 0; sI != sE; ++sI, ++i)
+ for(; sI != sE; ++sI)
{
- if((*sI) != NULL)
+ if((*sI).second != NULL)
{
FWARNING(("FieldContainerFactoryBase::dump: "
- "Entry [%d] is not NULL ([%p]). \n", i, *sI));
+ "Entry [%d] is not NULL ([%p]). \n",
+ (*sI).first, (*sI).second));
- for(UInt32 j = 0; j < (*sI)->getNumAspects(); ++j)
+ for(UInt32 j = 0; j < (*sI).second->getNumAspects(); ++j)
{
- if((*sI)->getPtr(j) != NULL)
+ if((*sI).second->getPtr(j) != NULL)
{
FWARNING((" [%d] [%p] [%s] [%d %d]\n",
j,
- (*sI)->getPtr(j),
- (*sI)->getPtr(j)->getType().getCName(),
- (*sI)->getPtr(j)->getRefCount(),
- (*sI)->getPtr(j)->getWeakRefCount() ));
+ (*sI).second->getPtr(j),
+ (*sI).second->getPtr(j)->getType().getCName(),
+ (*sI).second->getPtr(j)->getRefCount(),
+ (*sI).second->getPtr(j)->getWeakRefCount() ));
}
else
{
FWARNING((" [%d] [%p] [] [N/A N/A]\n",
j,
- (*sI)->getPtr(j)));
+ (*sI).second->getPtr(j)));
}
}
}
diff --git a/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.h
b/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.h
index c222da5..8916a58 100644
--- a/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.h
+++ b/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.h
@@ -43,6 +43,7 @@
#endif
#include "OSGBaseTypes.h"
+#include "OSGDeprecatedCPP.h"
#include "OSGSingletonHolder.h"
#include "OSGContainerForwards.h"
#include "OSGTypeBase.h"
@@ -50,7 +51,6 @@
#include "OSGAspectStore.h"
#include "OSGContainerIdMapper.h"
-//#include "OSGFieldContainer.h"
#include <deque>
@@ -130,8 +130,9 @@ class OSG_BASE_DLLMAPPING FieldContainerFactoryBase :
typedef FieldContainer *ContainerHandlerP;
#endif
- typedef std::deque<ContainerHandlerP> ContainerStore;
- typedef ContainerStore::iterator ContainerStoreIt;
+ typedef OSG_HASH_MAP(UInt32, ContainerHandlerP) ContainerStore;
+ typedef ContainerStore::iterator ContainerStoreIt;
+ typedef ContainerStore::const_iterator ContainerStoreConstIt;
/*---------------------------------------------------------------------*/
/*! \name dcast */
@@ -171,12 +172,23 @@ class OSG_BASE_DLLMAPPING FieldContainerFactoryBase :
ContainerPtr getMappedContainer (UInt32 uiContainerId) const;
Int32 findContainer (ContainerPtr ptr ) const;
-
+
/*! \} */
/*---------------------------------------------------------------------*/
/*! \name Get */
/*! \{ */
+ void lockStore (void);
+ void unlockStore(void);
+
+ ContainerStoreConstIt beginStore (void) const;
+ ContainerStoreConstIt endStore (void) const;
+
+ /*! \} */
+ /*---------------------------------------------------------------------*/
+ /*! \name Registration */
+ /*! \{ */
+
UInt32 registerContainer (const ContainerPtr &pContainer );
bool deregisterContainer(const UInt32 uiContainerId);
@@ -246,7 +258,8 @@ class OSG_BASE_DLLMAPPING FieldContainerFactoryBase :
LockRefPtr _pStoreLock;
- ContainerStore _vContainerStore;
+ UInt32 _nextContainerId;
+ ContainerStore _containerStore;
/*! Currently active field container mapper. */
ContainerIdMapper *_pMapper;
diff --git a/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.inl
b/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.inl
index f66fb1a..4771a85 100644
--- a/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.inl
+++ b/Source/Base/FieldContainer/Base/OSGFieldContainerFactory.inl
@@ -52,7 +52,7 @@ UInt32 FieldContainerFactoryBase::getNumContainers(void) const
_pStoreLock->acquire();
- returnValue = _vContainerStore.size();
+ returnValue = _containerStore.size();
_pStoreLock->release();
@@ -67,16 +67,15 @@ FieldContainerFactoryBase::ContainerPtr
_pStoreLock->acquire();
- if(uiContainerId < _vContainerStore.size())
+ ContainerStoreConstIt sI = _containerStore.find(uiContainerId);
+
+ if(sI != _containerStore.end())
{
- if(_vContainerStore[uiContainerId] != NULL)
- {
#ifdef OSG_MT_CPTR_ASPECT
- returnValue = _vContainerStore[uiContainerId]->getPtr();
+ returnValue = (*sI).second->getPtr();
#else
- returnValue = _vContainerStore[uiContainerId];
+ returnValue = (*sI).second;
#endif
- }
}
_pStoreLock->release();
@@ -92,9 +91,11 @@ FieldContainerFactoryBase::ContainerHandlerP
_pStoreLock->acquire();
- if(uiContainerId < _vContainerStore.size())
+ ContainerStoreConstIt sI = _containerStore.find(uiContainerId);
+
+ if(sI != _containerStore.end())
{
- returnValue = _vContainerStore[uiContainerId];
+ returnValue = (*sI).second;
}
_pStoreLock->release();
@@ -123,32 +124,45 @@ FieldContainerFactoryBase::ContainerPtr
}
}
-
+/*! Lock the container store to allow iteration over it.
+ \warning This makes it impossible to create new containers,
+ be sure you know what you are doing!
+ */
inline
-bool FieldContainerFactoryBase::deregisterContainer(const UInt32 uiContainerId)
+void FieldContainerFactoryBase::lockStore(void)
{
- bool returnValue = false;
-
_pStoreLock->acquire();
+}
- if(uiContainerId < _vContainerStore.size())
- {
- _vContainerStore[uiContainerId] = NULL;
- }
-#ifdef OSG_DEBUG
- else
- {
- FWARNING(("FieldContainerFactory::unregisterFieldContainer:"
- "id %d inconsistent with store size %"PRISize"!\n",
- uiContainerId,
- _vContainerStore.size()));
- returnValue = true;
- }
-#endif
-
+/*! Unlock the container store.
+ \warning Be sure you know what you are doing!
+ */
+inline
+void FieldContainerFactoryBase::unlockStore(void)
+{
_pStoreLock->release();
+}
- return returnValue;
+/*! Returns a begin iterator over the container storage.
+ The container storage should be locked with lockStore before
+ iterating over it.
+*/
+inline
+FieldContainerFactoryBase::ContainerStoreConstIt
+FieldContainerFactoryBase::beginStore(void) const
+{
+ return _containerStore.begin();
+}
+
+/*! Returns an end iterator over the container storage.
+ The container storage should be locked with lockStore before
+ iterating over it.
+*/
+inline
+FieldContainerFactoryBase::ContainerStoreConstIt
+FieldContainerFactoryBase::endStore(void) const
+{
+ return _containerStore.end();
}
OSG_END_NAMESPACE
diff --git a/Source/System/FieldContainer/Misc/OSGFieldContainerUtils.cpp
b/Source/System/FieldContainer/Misc/OSGFieldContainerUtils.cpp
index 18daaf0..453fc5f 100644
--- a/Source/System/FieldContainer/Misc/OSGFieldContainerUtils.cpp
+++ b/Source/System/FieldContainer/Misc/OSGFieldContainerUtils.cpp
@@ -296,11 +296,16 @@ bool compareContainerEqual(
void MemoryConsumption::scan(void)
{
- UInt32 numCont = FieldContainerFactory::the()->getNumContainers();
+ FieldContainerFactory::the()->lockStore();
- for(UInt32 i = 0; i < numCont; ++i)
+ FieldContainerFactoryBase::ContainerStoreConstIt sIt =
+ FieldContainerFactory::the()->beginStore();
+ FieldContainerFactoryBase::ContainerStoreConstIt sEnd =
+ FieldContainerFactory::the()->endStore();
+
+ for(; sIt != sEnd; ++sIt)
{
- FieldContainer *pFC = FieldContainerFactory::the()->getContainer(i);
+ FieldContainer *pFC = (*sIt).second->getPtr();
if(pFC == NULL)
continue;
@@ -318,6 +323,8 @@ void MemoryConsumption::scan(void)
_memMap[pFC->getType().getId()] = MemCountPair(binSize, 1);
}
}
+
+ FieldContainerFactory::the()->unlockStore();
}
------------------------------------------------------------------------------
The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE:
Pinpoint memory and threading errors before they happen.
Find and fix more than 250 security defects in the development cycle.
Locate bottlenecks in serial and parallel code that limit performance.
http://p.sf.net/sfu/intel-dev2devfeb
_______________________________________________
Opensg-users mailing list
Opensg-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensg-users
