Currently job artifacts in CI/CD pipelines on LRZ GitLab never expire. Starting from Wed 26.1.2022 the default expiration time will be 30 days (GitLab default). Currently existing artifacts in already completed jobs will not be affected by the change. The latest artifacts for all jobs in the latest successful pipelines will be kept. More information: https://gitlab.lrz.de/help/user/admin_area/settings/continuous_integration.html#default-artifacts-expiration

Commit 694a519e authored by schultezub's avatar schultezub
Browse files

Various changes to improve threading behaviour and coding conveniene:

 * Moved destruction of ReferenceCounted object to seperate thread (thus unloading rendering thread)
 * integration of OpenGL garbage collector into OpenGLJobProcessor (finally, you don't need to care anymore whether your code might call a GL object destructor and hence needs an OpenGL context)
 * changed DataContainer implementation from mutex/lock to concurrent container
 * fixing improper order of GL object deinitilization on program exit

git-svn-id: https://camplinux.in.tum.de/svn/campvis/trunk@492 bb408c1c-ae56-11e1-83d9-df6b3e0c105e
parent 46a9e6cd
...@@ -183,9 +183,6 @@ namespace campvis { ...@@ -183,9 +183,6 @@ namespace campvis {
_mainWindow->deinit(); _mainWindow->deinit();
// deinit OpenGL and tgt
tgt::deinitGL();
QuadRenderer::deinit(); QuadRenderer::deinit();
#ifdef HAS_KISSCL #ifdef HAS_KISSCL
...@@ -193,6 +190,10 @@ namespace campvis { ...@@ -193,6 +190,10 @@ namespace campvis {
kisscl::CLRuntime::deinit(); kisscl::CLRuntime::deinit();
} }
#endif #endif
// deinit OpenGL and tgt
tgt::deinitGL();
} }
SimpleJobProcessor::deinit(); SimpleJobProcessor::deinit();
......
...@@ -100,6 +100,7 @@ namespace campvis { ...@@ -100,6 +100,7 @@ namespace campvis {
_dataContainer->s_dataAdded.disconnect(this); _dataContainer->s_dataAdded.disconnect(this);
} }
_handles.clear();
GLJobProc.deregisterContext(this); GLJobProc.deregisterContext(this);
ShdrMgr.dispose(_paintShader); ShdrMgr.dispose(_paintShader);
delete _quad; delete _quad;
...@@ -257,8 +258,9 @@ namespace campvis { ...@@ -257,8 +258,9 @@ namespace campvis {
_texturesDirty = true; _texturesDirty = true;
} }
} }
invalidate(); if (_texturesDirty)
invalidate();
} }
void DataContainerInspectorCanvas::setDataHandles(const std::vector< std::pair<QString, QtDataHandle> >& handles) { void DataContainerInspectorCanvas::setDataHandles(const std::vector< std::pair<QString, QtDataHandle> >& handles) {
......
...@@ -193,7 +193,7 @@ namespace campvis { ...@@ -193,7 +193,7 @@ namespace campvis {
_lblName->setText(QString::number(handles.size()) + " DataHandles selected"); _lblName->setText(QString::number(handles.size()) + " DataHandles selected");
_lblTimestamp->setText("Timestamp: n/a"); _lblTimestamp->setText("Timestamp: n/a");
_canvas->p_transferFunction.getTF()->setImageHandle(0); _canvas->p_transferFunction.getTF()->setImageHandle(DataHandle(0));
} }
_lblLocalMemoryFootprint->setText("Local Memory Footprint: " + humanizeBytes(_localFootprint)); _lblLocalMemoryFootprint->setText("Local Memory Footprint: " + humanizeBytes(_localFootprint));
_lblVideoMemoryFootprint->setText("Video Memory Footprint: " + humanizeBytes(_videoFootprint)); _lblVideoMemoryFootprint->setText("Video Memory Footprint: " + humanizeBytes(_videoFootprint));
...@@ -229,6 +229,13 @@ namespace campvis { ...@@ -229,6 +229,13 @@ namespace campvis {
_canvas->deinit(); _canvas->deinit();
_pcWidget->updatePropCollection(0); _pcWidget->updatePropCollection(0);
if (_dataContainer != 0) {
_dataContainer->s_dataAdded.disconnect(this);
}
_dataContainer = 0;
_dctWidget->update(0);
} }
void DataContainerInspectorWidget::onDCTWidgetSelectionModelSelectionChanged(const QItemSelection& selected, const QItemSelection& deselected) { void DataContainerInspectorWidget::onDCTWidgetSelectionModelSelectionChanged(const QItemSelection& selected, const QItemSelection& deselected) {
......
...@@ -221,10 +221,12 @@ namespace campvis { ...@@ -221,10 +221,12 @@ namespace campvis {
delete _rootItem; delete _rootItem;
_rootItem = new DataContainerTreeRootItem(); _rootItem = new DataContainerTreeRootItem();
std::vector< std::pair< std::string, DataHandle> > handlesCopy = dataContainer->getDataHandlesCopy(); if (dataContainer != 0) {
for (std::vector< std::pair< std::string, DataHandle> >::iterator it = handlesCopy.begin(); it != handlesCopy.end(); ++it) { std::vector< std::pair< std::string, DataHandle> > handlesCopy = dataContainer->getDataHandlesCopy();
DataHandleTreeItem* dhti = new DataHandleTreeItem(QtDataHandle(it->second), it->first, _rootItem); for (std::vector< std::pair< std::string, DataHandle> >::iterator it = handlesCopy.begin(); it != handlesCopy.end(); ++it) {
_itemMap.insert(std::make_pair(QString::fromStdString(it->first), dhti)); DataHandleTreeItem* dhti = new DataHandleTreeItem(QtDataHandle(it->second), it->first, _rootItem);
_itemMap.insert(std::make_pair(QString::fromStdString(it->first), dhti));
}
} }
} }
......
...@@ -83,6 +83,7 @@ namespace campvis { ...@@ -83,6 +83,7 @@ namespace campvis {
void AbstractTransferFunction::deinit() { void AbstractTransferFunction::deinit() {
delete _texture; delete _texture;
_texture = 0; _texture = 0;
_imageHandle = DataHandle(0);
} }
void AbstractTransferFunction::bind(tgt::Shader* shader, const tgt::TextureUnit& texUnit, const std::string& transFuncUniform /*= "_transferFunction"*/, const std::string& transFuncParamsUniform /*= "_transferFunctionParameters"*/) { void AbstractTransferFunction::bind(tgt::Shader* shader, const tgt::TextureUnit& texUnit, const std::string& transFuncUniform /*= "_transferFunction"*/, const std::string& transFuncParamsUniform /*= "_transferFunctionParameters"*/) {
......
...@@ -53,13 +53,13 @@ namespace campvis { ...@@ -53,13 +53,13 @@ namespace campvis {
void DataContainer::addDataHandle(const std::string& name, const DataHandle& dh) { void DataContainer::addDataHandle(const std::string& name, const DataHandle& dh) {
tgtAssert(dh.getData() != 0, "The data in the DataHandle must not be 0!"); tgtAssert(dh.getData() != 0, "The data in the DataHandle must not be 0!");
{ {
tbb::spin_mutex::scoped_lock lock(_localMutex); //tbb::spin_mutex::scoped_lock lock(_localMutex);
std::map<std::string, DataHandle>::iterator it = _handles.lower_bound(name); tbb::concurrent_unordered_map<std::string, DataHandle>::iterator it = _handles.find(name);
if (it != _handles.end() && it->first == name) { if (it != _handles.end()/* && it->first == name*/) {
it->second = dh; it->second = dh;
} }
else { else {
_handles.insert(it, std::make_pair(name, dh)); _handles.insert(/*it,*/ std::make_pair(name, dh));
} }
} }
s_dataAdded(name, dh); s_dataAdded(name, dh);
...@@ -67,13 +67,13 @@ namespace campvis { ...@@ -67,13 +67,13 @@ namespace campvis {
} }
bool DataContainer::hasData(const std::string& name) const { bool DataContainer::hasData(const std::string& name) const {
tbb::spin_mutex::scoped_lock lock(_localMutex); //tbb::spin_mutex::scoped_lock lock(_localMutex);
return (_handles.find(name) != _handles.end()); return (_handles.find(name) != _handles.end());
} }
DataHandle DataContainer::getData(const std::string& name) const { DataHandle DataContainer::getData(const std::string& name) const {
tbb::spin_mutex::scoped_lock lock(_localMutex); //tbb::spin_mutex::scoped_lock lock(_localMutex);
std::map<std::string, DataHandle>::const_iterator it = _handles.find(name); tbb::concurrent_unordered_map<std::string, DataHandle>::const_iterator it = _handles.find(name);
if (it == _handles.end()) if (it == _handles.end())
return DataHandle(0); return DataHandle(0);
else else
...@@ -82,9 +82,9 @@ namespace campvis { ...@@ -82,9 +82,9 @@ namespace campvis {
void DataContainer::removeData(const std::string& name) { void DataContainer::removeData(const std::string& name) {
tbb::spin_mutex::scoped_lock lock(_localMutex); tbb::spin_mutex::scoped_lock lock(_localMutex);
std::map<std::string, DataHandle>::iterator it = _handles.find(name); tbb::concurrent_unordered_map<std::string, DataHandle>::const_iterator it = _handles.find(name);
if (it != _handles.end()) { if (it != _handles.end()) {
_handles.erase(it); _handles.unsafe_erase(it);
} }
} }
...@@ -93,16 +93,21 @@ namespace campvis { ...@@ -93,16 +93,21 @@ namespace campvis {
toReturn.reserve(_handles.size()); toReturn.reserve(_handles.size());
tbb::spin_mutex::scoped_lock lock(_localMutex); tbb::spin_mutex::scoped_lock lock(_localMutex);
for (std::map<std::string, DataHandle>::const_iterator it = _handles.begin(); it != _handles.end(); ++it) { for (tbb::concurrent_unordered_map<std::string, DataHandle>::const_iterator it = _handles.begin(); it != _handles.end(); ++it) {
toReturn.push_back(std::make_pair(it->first, it->second)); toReturn.push_back(std::make_pair(it->first, it->second));
} }
return toReturn; return toReturn;
} }
std::map<std::string, DataHandle> DataContainer::getHandlesCopy() const { // tbb::concurrent_unordered_map<std::string, DataHandle> DataContainer::getHandlesCopy() const {
tbb::spin_mutex::scoped_lock lock(_localMutex); // //tbb::spin_mutex::scoped_lock lock(_localMutex);
return _handles; // return _handles;
// }
void DataContainer::clear() {
//tbb::spin_mutex::scoped_lock lock(_localMutex);
_handles.clear();
} }
} }
\ No newline at end of file
...@@ -31,6 +31,7 @@ ...@@ -31,6 +31,7 @@
#define DATACONTAINER_H__ #define DATACONTAINER_H__
#include "sigslot/sigslot.h" #include "sigslot/sigslot.h"
#include "tbb/concurrent_unordered_map.h"
#include "tbb/spin_mutex.h" #include "tbb/spin_mutex.h"
#include "core/datastructures/datahandle.h" #include "core/datastructures/datahandle.h"
...@@ -170,6 +171,12 @@ namespace campvis { ...@@ -170,6 +171,12 @@ namespace campvis {
*/ */
void removeData(const std::string& name); void removeData(const std::string& name);
/**
* Removes all DataHandles from this DataContainer.
* \note This method is \b NOT thread-safe!
*/
void clear();
/** /**
* Returns a copy of the current list of DataHandles. * Returns a copy of the current list of DataHandles.
* \note Use with caution, this method is to be considered as slow, as it includes several * \note Use with caution, this method is to be considered as slow, as it includes several
...@@ -184,7 +191,7 @@ namespace campvis { ...@@ -184,7 +191,7 @@ namespace campvis {
* copies and locks the whole DataContainer during execution. * copies and locks the whole DataContainer during execution.
* \return A copy of the current handles map. The caller has to take ownership of the passed pointers! * \return A copy of the current handles map. The caller has to take ownership of the passed pointers!
*/ */
std::map<std::string, DataHandle> getHandlesCopy() const; //std::map<std::string, DataHandle> getHandlesCopy() const;
/** /**
* Signal emitted when data has been added to this DataContainer (this includes also data being replaced). * Signal emitted when data has been added to this DataContainer (this includes also data being replaced).
...@@ -197,9 +204,11 @@ namespace campvis { ...@@ -197,9 +204,11 @@ namespace campvis {
private: private:
/// Map of the DataHandles in this collection and their IDs. The DataHandles contain valid data. /// Map of the DataHandles in this collection and their IDs. The DataHandles contain valid data.
std::map<std::string, DataHandle> _handles; //std::map<std::string, DataHandle> _handles;
mutable tbb::spin_mutex _localMutex; mutable tbb::spin_mutex _localMutex;
tbb::concurrent_unordered_map<std::string, DataHandle> _handles;
static const std::string loggerCat_; static const std::string loggerCat_;
}; };
......
...@@ -58,7 +58,7 @@ namespace campvis { ...@@ -58,7 +58,7 @@ namespace campvis {
* counting mechanism. Make sure not to interfere with it or delete \a data yourself! * counting mechanism. Make sure not to interfere with it or delete \a data yourself!
* \param data Data for the DataHandle * \param data Data for the DataHandle
*/ */
DataHandle(AbstractData* data); explicit DataHandle(AbstractData* data = 0);
/** /**
* Copy-constructor * Copy-constructor
......
...@@ -60,8 +60,12 @@ namespace campvis { ...@@ -60,8 +60,12 @@ namespace campvis {
GeometryData::~GeometryData() { GeometryData::~GeometryData() {
if (_buffersInitialized) { if (_buffersInitialized) {
std::vector<tgt::BufferObject*> buffers(_buffers, _buffers + NUM_BUFFERS); delete _verticesBuffer;
GLJobProc.enqueueJob(_context, makeJobOnHeap(&GeometryData::deleteBuffers, buffers), OpenGLJobProcessor::LowPriorityJob); delete _texCoordsBuffer;
delete _colorsBuffer;
delete _normalsBuffer;
// std::vector<tgt::BufferObject*> buffers(_buffers, _buffers + NUM_BUFFERS);
// GLJobProc.enqueueJob(_context, makeJobOnHeap(&GeometryData::deleteBuffers, buffers), OpenGLJobProcessor::LowPriorityJob);
} }
} }
...@@ -73,8 +77,12 @@ namespace campvis { ...@@ -73,8 +77,12 @@ namespace campvis {
// delete old VBOs and null pointers // delete old VBOs and null pointers
if (_buffersInitialized) { if (_buffersInitialized) {
std::vector<tgt::BufferObject*> buffers(_buffers, _buffers + NUM_BUFFERS); delete _verticesBuffer;
GLJobProc.enqueueJob(_context, makeJobOnHeap(&GeometryData::deleteBuffers, buffers), OpenGLJobProcessor::LowPriorityJob); delete _texCoordsBuffer;
delete _colorsBuffer;
delete _normalsBuffer;
// std::vector<tgt::BufferObject*> buffers(_buffers, _buffers + NUM_BUFFERS);
// GLJobProc.enqueueJob(_context, makeJobOnHeap(&GeometryData::deleteBuffers, buffers), OpenGLJobProcessor::LowPriorityJob);
} }
_verticesBuffer = 0; _verticesBuffer = 0;
......
...@@ -114,11 +114,12 @@ namespace campvis { ...@@ -114,11 +114,12 @@ namespace campvis {
} }
ImageRepresentationRenderTarget::~ImageRepresentationRenderTarget() { ImageRepresentationRenderTarget::~ImageRepresentationRenderTarget() {
if (_fbo != 0) { // TODO: double check whether deleting FBOs without detaching their attachments is not harmful
_fbo->activate(); // if (_fbo != 0) {
_fbo->detachAll(); // _fbo->activate();
_fbo->deactivate(); // _fbo->detachAll();
} // _fbo->deactivate();
// }
delete _fbo; delete _fbo;
for (std::vector<tgt::Texture*>::iterator it = _colorTextures.begin(); it != _colorTextures.end(); ++it) for (std::vector<tgt::Texture*>::iterator it = _colorTextures.begin(); it != _colorTextures.end(); ++it)
......
...@@ -76,6 +76,9 @@ namespace campvis { ...@@ -76,6 +76,9 @@ namespace campvis {
LERROR("Caught Exception during deinitialization of processor: " << e.what()); LERROR("Caught Exception during deinitialization of processor: " << e.what());
} }
} }
// clear DataContainer
_data.clear();
} }
void AbstractPipeline::onPropertyChanged(const AbstractProperty* prop) { void AbstractPipeline::onPropertyChanged(const AbstractProperty* prop) {
...@@ -122,7 +125,7 @@ namespace campvis { ...@@ -122,7 +125,7 @@ namespace campvis {
processor->unlockProcessor(); processor->unlockProcessor();
#ifdef CAMPVIS_DEBUG #ifdef CAMPVIS_DEBUG
LDEBUG("Executed processor " << processor->getName() << " duration: " << (endTime - startTime)); //LDEBUG("Executed processor " << processor->getName() << " duration: " << (endTime - startTime));
#endif #endif
} }
} }
......
...@@ -30,6 +30,8 @@ ...@@ -30,6 +30,8 @@
#include "opengljobprocessor.h" #include "opengljobprocessor.h"
#include "tgt/assert.h" #include "tgt/assert.h"
#include "tgt/logmanager.h"
#include "tgt/openglgarbagecollector.h"
#include "tgt/qt/qtcontextmanager.h" #include "tgt/qt/qtcontextmanager.h"
#include "core/tools/job.h" #include "core/tools/job.h"
...@@ -57,12 +59,11 @@ namespace campvis { ...@@ -57,12 +59,11 @@ namespace campvis {
void OpenGLJobProcessor::stop() { void OpenGLJobProcessor::stop() {
_stopExecution = true; _stopExecution = true;
_evaluationCondition.notify_all(); _evaluationCondition.notify_all();
Runnable::stop();
} }
void OpenGLJobProcessor::run() { void OpenGLJobProcessor::run() {
std::unique_lock<tbb::mutex> lock(CtxtMgr.getGlMutex()); std::unique_lock<tbb::mutex> lock(CtxtMgr.getGlMutex());
clock_t lastCleanupTime = clock() * 1000 / CLOCKS_PER_SEC;
while (! _stopExecution) { while (! _stopExecution) {
// this is a simple round-robing scheduling between all contexts: // this is a simple round-robing scheduling between all contexts:
...@@ -123,6 +124,13 @@ namespace campvis { ...@@ -123,6 +124,13 @@ namespace campvis {
jobToDo->execute(); jobToDo->execute();
delete jobToDo; delete jobToDo;
} }
// fourth: start the GC if it's time
if (clock() * 1000 / CLOCKS_PER_SEC - lastCleanupTime > 250) {
GLGC.deleteGarbage();
lastCleanupTime = clock();
}
} }
while (_pause > 0) { while (_pause > 0) {
......
...@@ -171,7 +171,7 @@ namespace campvis { ...@@ -171,7 +171,7 @@ namespace campvis {
* \return (_serialJobs.empty() && _lowPriorityJobs.empty() && (_paintJob == 0)) * \return (_serialJobs.empty() && _lowPriorityJobs.empty() && (_paintJob == 0))
*/ */
bool empty() const { bool empty() const {
return (_serialJobs.empty() && _lowPriorityJobs.empty() && (_paintJob == 0)); return ((_paintJob == 0) && _serialJobs.empty() && _lowPriorityJobs.empty());
} }
}; };
......
...@@ -28,6 +28,8 @@ ...@@ -28,6 +28,8 @@
// ================================================================================================ // ================================================================================================
#include "referencecounted.h" #include "referencecounted.h"
#include "core/tools/job.h"
#include "core/tools/simplejobprocessor.h"
namespace campvis { namespace campvis {
ReferenceCounted::ReferenceCounted() ReferenceCounted::ReferenceCounted()
...@@ -55,7 +57,7 @@ namespace campvis { ...@@ -55,7 +57,7 @@ namespace campvis {
void ReferenceCounted::removeReference() { void ReferenceCounted::removeReference() {
if (--_refCount == 0) if (--_refCount == 0)
delete this; SimpleJobProc.enqueueJob(makeJob(&ReferenceCounted::deleteInstance, this));
} }
void ReferenceCounted::markUnsharable() { void ReferenceCounted::markUnsharable() {
...@@ -70,5 +72,9 @@ namespace campvis { ...@@ -70,5 +72,9 @@ namespace campvis {
return _refCount > 1; return _refCount > 1;
} }
void ReferenceCounted::deleteInstance(ReferenceCounted* instance) {
delete instance;
}
} }
...@@ -91,6 +91,8 @@ namespace campvis { ...@@ -91,6 +91,8 @@ namespace campvis {
*/ */
bool isShared() const; bool isShared() const;
static void deleteInstance(ReferenceCounted* instance);
private: private:
tbb::atomic<size_t> _refCount; ///< number of references to this object tbb::atomic<size_t> _refCount; ///< number of references to this object
bool _shareable; ///< flag whether this object is shareable, i.e. no non-const pointers to this object exist bool _shareable; ///< flag whether this object is shareable, i.e. no non-const pointers to this object exist
......
...@@ -41,18 +41,26 @@ namespace campvis { ...@@ -41,18 +41,26 @@ namespace campvis {
, _thread(0) , _thread(0)
{ {
_stopExecution = false; _stopExecution = false;
_running = false;
} }
Runnable::~Runnable() { Runnable::~Runnable() {
stop(); if (_running)
delete _thread; stop();
delete _thread;
} }
void Runnable::stop() { void Runnable::stop() {
if (!_running || _thread == 0)
return;
_stopExecution = true; _stopExecution = true;
try { try {
if (_thread->joinable()) if (_thread->joinable())
_thread->join(); _thread->join();
_running = false;
} }
catch(std::exception& e) { catch(std::exception& e) {
LERRORC("CAMPVis.core.tools.Runnable", "Caught exception during _thread.join: " << e.what()); LERRORC("CAMPVis.core.tools.Runnable", "Caught exception during _thread.join: " << e.what());
...@@ -61,6 +69,7 @@ namespace campvis { ...@@ -61,6 +69,7 @@ namespace campvis {
void Runnable::start() { void Runnable::start() {
_thread = new std::thread(&invokeThread, this); _thread = new std::thread(&invokeThread, this);
_running = true;
} }
} }
...@@ -79,6 +79,7 @@ namespace campvis { ...@@ -79,6 +79,7 @@ namespace campvis {
Runnable& operator =(Runnable const&); Runnable& operator =(Runnable const&);
std::thread* _thread; ///< Thread of the Runnable std::thread* _thread; ///< Thread of the Runnable
tbb::atomic<bool> _running;
}; };
} }
......
Markdown is supported
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