From 8d40b326311c5297ce636cf2e315f8ab70665dc1 Mon Sep 17 00:00:00 2001 From: Bradley Lowekamp Date: Mon, 5 Mar 2018 11:44:08 -0500 Subject: [PATCH 1/2] ENH: Add move support to the MetaDataDictionary object --- .../Common/include/itkMetaDataDictionary.h | 17 ++++++++++++- Modules/Core/Common/include/itkObject.h | 1 + .../Core/Common/src/itkMetaDataDictionary.cxx | 24 +++++++++++++++---- Modules/Core/Common/src/itkObject.cxx | 16 ++++++++++++- 4 files changed, 51 insertions(+), 7 deletions(-) diff --git a/Modules/Core/Common/include/itkMetaDataDictionary.h b/Modules/Core/Common/include/itkMetaDataDictionary.h index b909ca7fd5a..7a1d471bb1d 100644 --- a/Modules/Core/Common/include/itkMetaDataDictionary.h +++ b/Modules/Core/Common/include/itkMetaDataDictionary.h @@ -22,6 +22,7 @@ #include #include #include +#include namespace itk { @@ -57,8 +58,10 @@ class ITKCommon_EXPORT MetaDataDictionary MetaDataDictionary(); // Copy Constructor MetaDataDictionary(const MetaDataDictionary &); + MetaDataDictionary(MetaDataDictionary &&); // operator = MetaDataDictionary & operator=(const MetaDataDictionary &); + MetaDataDictionary & operator=(MetaDataDictionary &&); // Destructor virtual ~MetaDataDictionary(); @@ -112,8 +115,20 @@ class ITKCommon_EXPORT MetaDataDictionary /** remove all MetaObjects from dictionary */ void Clear(); + void Swap( MetaDataDictionary &other ) + { + using std::swap; + swap(m_Dictionary, other.m_Dictionary); + } + private: - MetaDataDictionaryMapType *m_Dictionary; + std::unique_ptr m_Dictionary; }; + +inline void swap(MetaDataDictionary &a, MetaDataDictionary &b ) +{ + a.Swap(b); +} + } #endif // itkMetaDataDictionary_h diff --git a/Modules/Core/Common/include/itkObject.h b/Modules/Core/Common/include/itkObject.h index 3691c272013..9861fadcd74 100644 --- a/Modules/Core/Common/include/itkObject.h +++ b/Modules/Core/Common/include/itkObject.h @@ -172,6 +172,7 @@ class ITKCommon_EXPORT Object:public LightObject * Set the MetaDataDictionary */ void SetMetaDataDictionary(const MetaDataDictionary & rhs); + void SetMetaDataDictionary( MetaDataDictionary && rrhs); /** * A facility to help application programmers set a diff --git a/Modules/Core/Common/src/itkMetaDataDictionary.cxx b/Modules/Core/Common/src/itkMetaDataDictionary.cxx index 997d61d949a..335d3865ffc 100644 --- a/Modules/Core/Common/src/itkMetaDataDictionary.cxx +++ b/Modules/Core/Common/src/itkMetaDataDictionary.cxx @@ -21,24 +21,28 @@ namespace itk { MetaDataDictionary ::MetaDataDictionary() + : m_Dictionary( new MetaDataDictionaryMapType() ) { - m_Dictionary = new MetaDataDictionaryMapType; } MetaDataDictionary ::~MetaDataDictionary() { - delete m_Dictionary; - m_Dictionary = nullptr; } MetaDataDictionary ::MetaDataDictionary(const MetaDataDictionary & old) + : m_Dictionary( new MetaDataDictionaryMapType(*old.m_Dictionary)) { - m_Dictionary = new MetaDataDictionaryMapType; - *m_Dictionary = *( old.m_Dictionary ); } +MetaDataDictionary +::MetaDataDictionary( MetaDataDictionary && rr) + : m_Dictionary( new MetaDataDictionaryMapType(std::move(*rr.m_Dictionary))) +{ +} + + MetaDataDictionary & MetaDataDictionary ::operator=(const MetaDataDictionary & old) { @@ -49,6 +53,16 @@ ::operator=(const MetaDataDictionary & old) return *this; } +MetaDataDictionary & MetaDataDictionary +::operator=(MetaDataDictionary && rr) +{ + if(this != &rr) + { + *m_Dictionary = std::move(*( rr.m_Dictionary )); + } + return *this; +} + void MetaDataDictionary ::Print(std::ostream & os) const diff --git a/Modules/Core/Common/src/itkObject.cxx b/Modules/Core/Common/src/itkObject.cxx index c7ccc0a6513..262dafaefb0 100644 --- a/Modules/Core/Common/src/itkObject.cxx +++ b/Modules/Core/Common/src/itkObject.cxx @@ -654,8 +654,22 @@ ::SetMetaDataDictionary(const MetaDataDictionary & rhs) { if ( m_MetaDataDictionary == nullptr ) { - m_MetaDataDictionary = new MetaDataDictionary; + m_MetaDataDictionary = new MetaDataDictionary(rhs); + return; } *m_MetaDataDictionary = rhs; } + +void +Object +::SetMetaDataDictionary( MetaDataDictionary && rrhs) +{ + if ( m_MetaDataDictionary == nullptr ) + { + m_MetaDataDictionary = new MetaDataDictionary(std::move(rrhs)); + return; + } + *m_MetaDataDictionary = std::move(rrhs); +} + } // end namespace itk From 7f5039af2ef22ea13c5bbe038aa2f8c58244fb42 Mon Sep 17 00:00:00 2001 From: Bradley Lowekamp Date: Mon, 5 Mar 2018 15:43:17 -0500 Subject: [PATCH 2/2] PERF: Implement copy on write for MetaDataDictionary The default behavior when reading a Image in ITK is to create 3 copies of the MetaDataDictionary for the ImageIO, ImageFileReader, and Image object. These are usually not modified, not shared and created by a deep copy. The internal std::map is shared via a std::share_ptr with a reference count. If the dictionaries are modified a deep copy is performed. --- .../Common/include/itkMetaDataDictionary.h | 19 +++++--- .../Core/Common/src/itkMetaDataDictionary.cxx | 43 ++++++++++++++++--- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/Modules/Core/Common/include/itkMetaDataDictionary.h b/Modules/Core/Common/include/itkMetaDataDictionary.h index 7a1d471bb1d..1db5b6bd7a5 100644 --- a/Modules/Core/Common/include/itkMetaDataDictionary.h +++ b/Modules/Core/Common/include/itkMetaDataDictionary.h @@ -36,6 +36,15 @@ namespace itk * classes, is designed to provide a mechanism for storing a collection of * arbitrary data types. The main motivation for such a collection is to * associate arbitrary data elements with itk DataObjects. + * + * The MetaDataDictionary implements shallow copying with copy on + * write behavior. When a copy of this class is created, the new copy + * will be shared with the old copy via C++11 shared pointers. When a + * non-constant operation is done, if the dictionary is not unique to + * this object, then a deep copy is performed. This make is very cheap + * to create multiple copies of the same dictionary if they are never + * modified. + * * \ingroup ITKCommon */ class ITKCommon_EXPORT MetaDataDictionary @@ -115,14 +124,12 @@ class ITKCommon_EXPORT MetaDataDictionary /** remove all MetaObjects from dictionary */ void Clear(); - void Swap( MetaDataDictionary &other ) - { - using std::swap; - swap(m_Dictionary, other.m_Dictionary); - } + void Swap( MetaDataDictionary &other ); private: - std::unique_ptr m_Dictionary; + bool MakeUnique(void); + + std::shared_ptr m_Dictionary; }; inline void swap(MetaDataDictionary &a, MetaDataDictionary &b ) diff --git a/Modules/Core/Common/src/itkMetaDataDictionary.cxx b/Modules/Core/Common/src/itkMetaDataDictionary.cxx index 335d3865ffc..e5b80ae790c 100644 --- a/Modules/Core/Common/src/itkMetaDataDictionary.cxx +++ b/Modules/Core/Common/src/itkMetaDataDictionary.cxx @@ -21,7 +21,7 @@ namespace itk { MetaDataDictionary ::MetaDataDictionary() - : m_Dictionary( new MetaDataDictionaryMapType() ) + : m_Dictionary( std::make_shared() ) { } @@ -32,13 +32,14 @@ MetaDataDictionary MetaDataDictionary ::MetaDataDictionary(const MetaDataDictionary & old) - : m_Dictionary( new MetaDataDictionaryMapType(*old.m_Dictionary)) + // perform shallow copy, so m_Dictionary is shared + : m_Dictionary( old.m_Dictionary ) { } MetaDataDictionary ::MetaDataDictionary( MetaDataDictionary && rr) - : m_Dictionary( new MetaDataDictionaryMapType(std::move(*rr.m_Dictionary))) + : m_Dictionary( std::move(rr.m_Dictionary) ) { } @@ -48,7 +49,8 @@ ::operator=(const MetaDataDictionary & old) { if(this != &old) { - *m_Dictionary = *( old.m_Dictionary ); + // perform shallow copy, so m_Dictionary is shared + m_Dictionary = old.m_Dictionary; } return *this; } @@ -58,7 +60,7 @@ ::operator=(MetaDataDictionary && rr) { if(this != &rr) { - *m_Dictionary = std::move(*( rr.m_Dictionary )); + m_Dictionary = std::move( rr.m_Dictionary ); } return *this; } @@ -67,6 +69,7 @@ void MetaDataDictionary ::Print(std::ostream & os) const { + os << "Dictionary use_count: " << m_Dictionary.use_count() << std::endl; for ( MetaDataDictionaryMapType::const_iterator it = m_Dictionary->begin(); it != m_Dictionary->end(); ++it ) @@ -74,12 +77,14 @@ ::Print(std::ostream & os) const os << ( *it ).first << " "; ( *it ).second->Print(os); } + } MetaDataObjectBase::Pointer & MetaDataDictionary ::operator[](const std::string & key) { + MakeUnique(); return ( *m_Dictionary )[key]; } @@ -110,6 +115,7 @@ void MetaDataDictionary ::Set(const std::string & key, MetaDataObjectBase * object) { + MakeUnique(); (*m_Dictionary)[key] = object; } @@ -140,6 +146,7 @@ MetaDataDictionary::Iterator MetaDataDictionary ::Begin() { + MakeUnique(); return m_Dictionary->begin(); } @@ -154,6 +161,7 @@ MetaDataDictionary::Iterator MetaDataDictionary ::End() { + MakeUnique(); return m_Dictionary->end(); } @@ -168,6 +176,7 @@ MetaDataDictionary::Iterator MetaDataDictionary ::Find(const std::string & key) { + MakeUnique(); return m_Dictionary->find(key); } @@ -182,9 +191,32 @@ void MetaDataDictionary ::Clear() { + MakeUnique(); this->m_Dictionary->clear(); } +void +MetaDataDictionary +::Swap( MetaDataDictionary &other ) +{ + using std::swap; + swap(m_Dictionary, other.m_Dictionary); +} + + +bool +MetaDataDictionary +::MakeUnique() +{ + if (m_Dictionary.use_count() > 1) + { + // copy the shared dictionary. + m_Dictionary = std::make_shared(*m_Dictionary); + return true; + } + return false; +} + bool MetaDataDictionary ::Erase( const std::string& key ) @@ -194,6 +226,7 @@ ::Erase( const std::string& key ) if( it != end ) { + MakeUnique(); m_Dictionary->erase( it ); return true; }