From 8ec177ed0de8868802fa659b252fbabb4ba1af1b Mon Sep 17 00:00:00 2001 From: Martin Heistermann Date: Wed, 13 Mar 2019 16:49:17 +0100 Subject: [PATCH] ResourceManager: implement move semantics, simplify logic Simplify property destruction logic and do not keep non-persistent props around in clearVec(). If a prop is alive but non-persistent, someone else has a shared_ptr to it, so we will not pull out the rug from under their feet. --- src/OpenVolumeMesh/Core/BaseProperty.hh | 14 ++---- src/OpenVolumeMesh/Core/PropertyPtrT_impl.hh | 5 +-- src/OpenVolumeMesh/Core/ResourceManager.cc | 45 ++++++++++++++----- src/OpenVolumeMesh/Core/ResourceManager.hh | 10 +++-- .../Core/ResourceManagerT_impl.hh | 36 +++++++-------- 5 files changed, 64 insertions(+), 46 deletions(-) diff --git a/src/OpenVolumeMesh/Core/BaseProperty.hh b/src/OpenVolumeMesh/Core/BaseProperty.hh index 3096396..2cf1818 100644 --- a/src/OpenVolumeMesh/Core/BaseProperty.hh +++ b/src/OpenVolumeMesh/Core/BaseProperty.hh @@ -55,9 +55,9 @@ class BaseProperty { public: friend class ResourceManager; - explicit BaseProperty(ResourceManager& _resMan) : resMan_(_resMan), lock_(false) {} + explicit BaseProperty(ResourceManager& _resMan) : resMan_(&_resMan) {} - BaseProperty(const BaseProperty& _cpy) : resMan_(_cpy.resMan_), lock_(_cpy.lock_) {} + BaseProperty(const BaseProperty& _cpy) : resMan_(_cpy.resMan_) {} BaseProperty& operator=(const BaseProperty& _cpy) = delete; @@ -95,15 +95,9 @@ protected: virtual void set_handle(const OpenVolumeMeshHandle& /*_handle*/) = 0; - void lock() { lock_ = true; } + void setResMan(ResourceManager *resMan) { resMan_ = resMan;} - void unlock() { lock_ = false; } - - bool locked() const { return lock_; } - - ResourceManager& resMan_; - - bool lock_; + ResourceManager* resMan_; }; } // Namespace OpenVolumeMesh diff --git a/src/OpenVolumeMesh/Core/PropertyPtrT_impl.hh b/src/OpenVolumeMesh/Core/PropertyPtrT_impl.hh index e31ff3c..fad8e86 100644 --- a/src/OpenVolumeMesh/Core/PropertyPtrT_impl.hh +++ b/src/OpenVolumeMesh/Core/PropertyPtrT_impl.hh @@ -54,9 +54,8 @@ PropertyPtr::~PropertyPtr() { * remove it, since the resource manager is the * only one who stores the property. */ - if(!locked() && !persistent() && ptr::shared_ptr::use_count() == 2) { - resMan_.release_property(PropHandleT(handle().idx())); - unlock(); + if(resMan_ && !persistent() && ptr::shared_ptr::use_count() == 2) { + resMan_->release_property(PropHandleT(handle().idx())); } } diff --git a/src/OpenVolumeMesh/Core/ResourceManager.cc b/src/OpenVolumeMesh/Core/ResourceManager.cc index c68a497..2b52644 100644 --- a/src/OpenVolumeMesh/Core/ResourceManager.cc +++ b/src/OpenVolumeMesh/Core/ResourceManager.cc @@ -32,22 +32,25 @@ * * \*===========================================================================*/ -/*===========================================================================*\ - * * - * $Revision$ * - * $Date$ * - * $LastChangedBy$ * - * * -\*===========================================================================*/ - #include "ResourceManager.hh" namespace OpenVolumeMesh { -ResourceManager::ResourceManager() { +ResourceManager::ResourceManager(const ResourceManager &other) +{ + *this = other; +} + +ResourceManager::ResourceManager(ResourceManager &&other) +{ + *this = std::move(other); } -ResourceManager::ResourceManager(const ResourceManager &other) { +ResourceManager& ResourceManager::operator=(const ResourceManager &other) +{ + if (this == &other) + return *this; + auto cloneProps = [this](const Properties &src, Properties &dest) { dest.reserve(src.size()); for (BaseProperty *bp: src) { @@ -61,6 +64,28 @@ ResourceManager::ResourceManager(const ResourceManager &other) { cloneProps(other.halfface_props_, halfface_props_); cloneProps(other.cell_props_, cell_props_); cloneProps(other.mesh_props_, mesh_props_); + return *this; +} + +ResourceManager& ResourceManager::operator=(ResourceManager &&other) +{ + if (this == &other) + return *this; + + auto moveProps = [this](Properties &&src, Properties &dest) { + dest = std::move(src); + for (auto prop: dest) { + prop->setResMan(this); + } + }; + moveProps(std::move(other.vertex_props_), vertex_props_); + moveProps(std::move(other.edge_props_), edge_props_); + moveProps(std::move(other.halfedge_props_), halfedge_props_); + moveProps(std::move(other.face_props_), face_props_); + moveProps(std::move(other.halfface_props_), halfface_props_); + moveProps(std::move(other.cell_props_), cell_props_); + moveProps(std::move(other.mesh_props_), mesh_props_); + return *this; } ResourceManager::~ResourceManager() { diff --git a/src/OpenVolumeMesh/Core/ResourceManager.hh b/src/OpenVolumeMesh/Core/ResourceManager.hh index 35e74d8..c0eb2ce 100644 --- a/src/OpenVolumeMesh/Core/ResourceManager.hh +++ b/src/OpenVolumeMesh/Core/ResourceManager.hh @@ -51,10 +51,11 @@ class BaseProperty; class ResourceManager { public: - ResourceManager(); + ResourceManager() = default; ResourceManager(const ResourceManager &other); - ResourceManager& operator=(const ResourceManager &other) = delete; - + ResourceManager(ResourceManager &&other); + ResourceManager& operator=(const ResourceManager &other); + ResourceManager& operator=(ResourceManager &&other); virtual ~ResourceManager(); template friend class PropertyPtr; @@ -319,6 +320,9 @@ private: template void clearVec(StdVecT& _vec); + template + void updatePropHandles(StdVecT& _vec); + Properties vertex_props_; Properties edge_props_; diff --git a/src/OpenVolumeMesh/Core/ResourceManagerT_impl.hh b/src/OpenVolumeMesh/Core/ResourceManagerT_impl.hh index bf7831c..0d09c6f 100644 --- a/src/OpenVolumeMesh/Core/ResourceManagerT_impl.hh +++ b/src/OpenVolumeMesh/Core/ResourceManagerT_impl.hh @@ -170,13 +170,11 @@ void ResourceManager::set_persistentT(PropT& _prop, bool _flag) { template void ResourceManager::remove_property(StdVecT& _vec, size_t _idx) { - (*(_vec.begin() + _idx))->lock(); - delete *(_vec.begin() + _idx); + auto prop_ptr = _vec[_idx]; + prop_ptr->setResMan(nullptr); + delete prop_ptr; _vec.erase(_vec.begin() + _idx); - size_t n = _vec.size(); - for(size_t i = 0; i < n; ++i) { - _vec[i]->set_handle(OpenVolumeMeshHandle((int)i)); - } + updatePropHandles(_vec); } template @@ -200,22 +198,20 @@ void ResourceManager::entity_deleted(StdVecT& _vec, const OpenVolumeMeshHandle& template void ResourceManager::clearVec(StdVecT& _vec) { - StdVecT newVec; - for(typename StdVecT::iterator it = _vec.begin(); - it != _vec.end(); ++it) { - if(!(*it)->persistent()) { -#ifndef NDEBUG - std::cerr << "Keeping property \"" << (*it)->name() - << "\" since it is still in use!" << std::endl; -#endif - (*it)->resize(0); - newVec.push_back(*it); - } - else - delete *it; + for (auto prop: _vec) { + prop->setResMan(nullptr); + delete prop; } + _vec.clear(); +} - _vec = newVec; +template +void ResourceManager::updatePropHandles(StdVecT &_vec) +{ + size_t n = _vec.size(); + for(size_t i = 0; i < n; ++i) { + _vec[i]->set_handle(OpenVolumeMeshHandle((int)i)); + } } } // Namespace OpenVolumeMesh -- GitLab