Meta data dictionary move and copy on write#455
Merged
blowekamp merged 5 commits intoInsightSoftwareConsortium:masterfrom Feb 1, 2019
Merged
Meta data dictionary move and copy on write#455blowekamp merged 5 commits intoInsightSoftwareConsortium:masterfrom
blowekamp merged 5 commits intoInsightSoftwareConsortium:masterfrom
Conversation
Member
Author
|
The is a revival of #98 |
Member
Author
|
As noted by @N-Dekker the I can not think of a good option to fix this interface. |
dzenanz
approved these changes
Jan 29, 2019
| MetaDataDictionary | ||
| ::Clear() | ||
| { | ||
| MakeUnique(); |
Member
There was a problem hiding this comment.
Will this create a copy, only to be erased by the next line?
Member
Author
|
This should be a WIP |
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.
Cover basic usage of the interface along with checking that a copy-on-write occurs at the proper time.
The constant implementation of MetaDataDictionary::operator[] might add an default initialized key if the key did not exist already.
Use Find and the resulting iterator to query existence and get value, instead of two separate searches.
blowekamp
force-pushed
the
MetaDataDictionaryMove
branch
from
25ae222 to
acd5e0d
Compare
blowekamp
commented
Jan 30, 2019
| { | ||
| MetaDataObjectBase::Pointer entry = ( *m_Dictionary )[key]; | ||
| const MetaDataObjectBase * constentry = entry.GetPointer(); | ||
| auto iter = m_Dictionary->find(key); |
Member
Author
There was a problem hiding this comment.
@N-Dekker I pondered deprecating this method, but instead I have just made it return a nullptr, and not modify the std::map when not does not exist. This const method does not exist in the std container, and the behavior is different than the non-constant method.
blowekamp
changed the title
hjmjohnson
approved these changes
Jan 31, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.