-
Notifications
You must be signed in to change notification settings - Fork 246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Core][PVS] Add PointerVectorSet::MutablePass to allow push_back safely. #12839
Open
sunethwarna
wants to merge
14
commits into
master
Choose a base branch
from
core/pvs/add_accessor
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+172
−100
Open
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
ab23b59
fix operators
sunethwarna 32d9d48
add static assert
sunethwarna 0fb1461
revert changes
sunethwarna 24e7167
add MutablePass
sunethwarna 8d6e0a2
Merge branch 'core/pvs/fix_operators' into core/pvs/add_accessor
sunethwarna 6510609
minor error message change
sunethwarna 1ee6ca2
adding some additional methods
sunethwarna 0b3d72a
minor
sunethwarna 78ac0ff
fix hdf5 application
sunethwarna ed74f0d
minor
sunethwarna d2ce128
remove std::atomic
sunethwarna c03c7b8
include parallel_utils header
sunethwarna 0ca518b
fix combine_model_part_modeler
sunethwarna 2aee55f
fix further cases which can break PVS
sunethwarna File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,8 @@ | |
#include <sstream> | ||
#include <cstddef> | ||
#include <utility> | ||
#include <type_traits> | ||
#include <type_traits> | ||
|
||
// External includes | ||
#include <boost/iterator/indirect_iterator.hpp> | ||
|
@@ -107,6 +109,99 @@ class PointerVectorSet final | |
using ptr_const_reverse_iterator = typename TContainerType::const_reverse_iterator; | ||
using difference_type = typename TContainerType::difference_type; | ||
|
||
///@} | ||
///@name Class definitions | ||
///@{ | ||
|
||
/// @brief Unrestricted accessor for the pointer vector set | ||
/// @details This class provides unrestricted access the underlying std::vector of the pointer vector | ||
/// set which may make the pointer vector set unsorted. Hence, once an object of this is created, the | ||
/// methods which utilize the sorted property of pointer vector set are frozen. At the destruction of this class, | ||
/// pointer vector set is sorted and made unique, and the frozen methods are released. | ||
/// @param | ||
/// @return | ||
class MutablePass | ||
{ | ||
///@name Life cycle | ||
///@{ | ||
|
||
// Constructor | ||
MutablePass(PointerVectorSet& rContainer) | ||
: mrContainer(rContainer) | ||
{ | ||
KRATOS_CRITICAL_SECTION | ||
KRATOS_ERROR_IF(mrContainer.mHasMutablePass) | ||
<< "A mutable pass is active. Hence, creation of an another MutablePass is prohibited."; | ||
mrContainer.mHasMutablePass = true; | ||
} | ||
|
||
///@} | ||
|
||
public: | ||
///@name Life cycle | ||
///@{ | ||
|
||
MutablePass(const MutablePass& rOther) = delete; | ||
|
||
MutablePass(MutablePass&& rOther) noexcept = default; | ||
|
||
// Destructor | ||
~MutablePass() | ||
{ | ||
// Sort the PointerVectorSet data | ||
std::sort(mrContainer.mData.begin(), mrContainer.mData.end(), CompareKey()); | ||
|
||
// Make the entities unique | ||
typename TContainerType::iterator new_end_it = std::unique(mrContainer.mData.begin(), mrContainer.mData.end(), EqualKeyTo()); | ||
|
||
// remove the duplicated entities. | ||
mrContainer.mData.erase(new_end_it, mrContainer.mData.end()); | ||
|
||
// unfreeze the methods which dependent on the sorted state of the PointerVectorSet | ||
mrContainer.mHasMutablePass = false; | ||
} | ||
|
||
///@name Public operators | ||
///@{ | ||
|
||
MutablePass& operator=(const MutablePass& rOther) = delete; | ||
|
||
///@} | ||
///@name Public methods | ||
///@{ | ||
|
||
void shrink_to_fit() | ||
{ | ||
mrContainer.mData.shrink_to_fit(); | ||
} | ||
|
||
void reserve(size_type NewCapacity) | ||
{ | ||
mrContainer.mData.reserve(NewCapacity); | ||
} | ||
|
||
void push_back(TPointerType pValue) | ||
{ | ||
mrContainer.mData.push_back(pValue); | ||
} | ||
|
||
///@} | ||
|
||
private: | ||
///@name Member variables | ||
///@{ | ||
|
||
PointerVectorSet& mrContainer; | ||
|
||
///@} | ||
///@name Friend classes | ||
///@{ | ||
|
||
friend class PointerVectorSet; | ||
|
||
///@} | ||
}; | ||
|
||
///@} | ||
///@name Life Cycle | ||
///@{ | ||
|
@@ -178,29 +273,15 @@ class PointerVectorSet final | |
*/ | ||
TDataType& operator[](const key_type& Key) | ||
{ | ||
ptr_iterator sorted_part_end; | ||
|
||
if (mData.size() - mSortedPartSize >= mMaxBufferSize) { | ||
Sort(); | ||
sorted_part_end = mData.end(); | ||
KRATOS_ERROR_IF(mHasMutablePass) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No go...you are adding an overhead to every access... |
||
<< "A mutable pass is active. Hence, use of PointerVectorSet::operator[] is prohibited."; | ||
ptr_iterator i(std::lower_bound(mData.begin(), mData.end(), Key, CompareKey())); | ||
if (i != mData.end() && EqualKeyTo(Key)(*i)) { | ||
return **i; | ||
} else { | ||
sorted_part_end = mData.begin() + mSortedPartSize; | ||
} | ||
|
||
ptr_iterator i(std::lower_bound(mData.begin(), sorted_part_end, Key, CompareKey())); | ||
if (i == sorted_part_end) { | ||
mSortedPartSize++; | ||
return **mData.insert(sorted_part_end, TPointerType(new TDataType(Key))); | ||
static_assert(!std::is_pointer_v<TPointerType>, "Raw pointers are not supported."); | ||
return **mData.insert(i, TPointerType(new TDataType(Key))); | ||
} | ||
|
||
if (!EqualKeyTo(Key)(*i)) { | ||
if ((i = std::find_if(sorted_part_end, mData.end(), EqualKeyTo(Key))) == mData.end()) { | ||
mData.push_back(TPointerType(new TDataType(Key))); | ||
return **(mData.end() - 1); | ||
} | ||
} | ||
|
||
return **i; | ||
} | ||
|
||
/** | ||
|
@@ -213,27 +294,15 @@ class PointerVectorSet final | |
*/ | ||
pointer& operator()(const key_type& Key) | ||
{ | ||
ptr_iterator sorted_part_end; | ||
|
||
if (mData.size() - mSortedPartSize >= mMaxBufferSize) { | ||
Sort(); | ||
sorted_part_end = mData.end(); | ||
} else | ||
sorted_part_end = mData.begin() + mSortedPartSize; | ||
|
||
ptr_iterator i(std::lower_bound(mData.begin(), sorted_part_end, Key, CompareKey())); | ||
if (i == sorted_part_end) { | ||
mSortedPartSize++; | ||
return *mData.insert(sorted_part_end, TPointerType(new TDataType(Key))); | ||
KRATOS_ERROR_IF(mHasMutablePass) | ||
<< "A mutable pass is active. Hence, use of PointerVectorSet::operator() is prohibited."; | ||
ptr_iterator i(std::lower_bound(mData.begin(), mData.end(), Key, CompareKey())); | ||
if (i != mData.end() && EqualKeyTo(Key)(*i)) { | ||
return *i; | ||
} else { | ||
static_assert(!std::is_pointer_v<TPointerType>, "Raw pointers are not supported."); | ||
return *mData.insert(i, TPointerType(new TDataType(Key))); | ||
} | ||
|
||
if (!EqualKeyTo(Key)(*i)) | ||
if ((i = std::find_if(sorted_part_end, mData.end(), EqualKeyTo(Key))) == mData.end()) { | ||
mData.push_back(TPointerType(new TDataType(Key))); | ||
return *(mData.end() - 1); | ||
} | ||
|
||
return *i; | ||
} | ||
|
||
/** | ||
|
@@ -568,6 +637,8 @@ class PointerVectorSet final | |
*/ | ||
iterator insert(const TPointerType& value) | ||
{ | ||
KRATOS_ERROR_IF(mHasMutablePass) | ||
<< "A mutable pass is active. Hence, use of PointerVectorSet::insert(value) is prohibited."; | ||
auto itr_pos = std::lower_bound(mData.begin(), mData.end(), KeyOf(*value), CompareKey()); | ||
if (itr_pos == mData.end()) { | ||
// the position to insert is at the end. | ||
|
@@ -596,6 +667,8 @@ class PointerVectorSet final | |
*/ | ||
iterator insert(const_iterator position_hint, const TPointerType& value) | ||
{ | ||
KRATOS_ERROR_IF(mHasMutablePass) | ||
<< "A mutable pass is active. Hence, use of PointerVectorSet::insert(position_hint, value) is prohibited."; | ||
if (empty()) { | ||
// the dataset is empty. So use push back. | ||
mData.push_back(value); | ||
|
@@ -647,6 +720,8 @@ class PointerVectorSet final | |
template <class InputIterator> | ||
void insert(InputIterator first, InputIterator last) | ||
{ | ||
KRATOS_ERROR_IF(mHasMutablePass) | ||
<< "A mutable pass is active. Hence, use of PointerVectorSet::insert(first, last) is prohibited."; | ||
// first sorts the input iterators and make the input unique. | ||
std::sort(first, last, CompareKey()); | ||
auto new_last = std::unique(first, last, EqualKeyTo()); | ||
|
@@ -741,20 +816,14 @@ class PointerVectorSet final | |
*/ | ||
iterator find(const key_type& Key) | ||
{ | ||
ptr_iterator sorted_part_end; | ||
|
||
if (mData.size() - mSortedPartSize >= mMaxBufferSize) { | ||
Sort(); | ||
sorted_part_end = mData.end(); | ||
} else | ||
sorted_part_end = mData.begin() + mSortedPartSize; | ||
|
||
ptr_iterator i(std::lower_bound(mData.begin(), sorted_part_end, Key, CompareKey())); | ||
if (i == sorted_part_end || (!EqualKeyTo(Key)(*i))) | ||
if ((i = std::find_if(sorted_part_end, mData.end(), EqualKeyTo(Key))) == mData.end()) | ||
return mData.end(); | ||
|
||
return i; | ||
KRATOS_ERROR_IF(mHasMutablePass) | ||
<< "A mutable pass is active. Hence, use of PointerVectorSet::find(key) is prohibited."; | ||
ptr_iterator i(std::lower_bound(mData.begin(), mData.end(), Key, CompareKey())); | ||
if (i != mData.end() && EqualKeyTo(Key)(*i)) { | ||
return i; | ||
} else { | ||
return mData.end(); | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -767,14 +836,14 @@ class PointerVectorSet final | |
*/ | ||
const_iterator find(const key_type& Key) const | ||
{ | ||
ptr_const_iterator sorted_part_end(mData.begin() + mSortedPartSize); | ||
|
||
ptr_const_iterator i(std::lower_bound(mData.begin(), sorted_part_end, Key, CompareKey())); | ||
if (i == sorted_part_end || (!EqualKeyTo(Key)(*i))) | ||
if ((i = std::find_if(sorted_part_end, mData.end(), EqualKeyTo(Key))) == mData.end()) | ||
return mData.end(); | ||
|
||
return const_iterator(i); | ||
KRATOS_ERROR_IF(mHasMutablePass) | ||
<< "A mutable pass is active. Hence, use of PointerVectorSet::find(key) is prohibited."; | ||
ptr_const_iterator i(std::lower_bound(mData.begin(), mData.end(), Key, CompareKey())); | ||
if (i != mData.end() && EqualKeyTo(Key)(*i)) { | ||
return const_iterator(i); | ||
} else { | ||
return mData.end(); | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -838,6 +907,11 @@ class PointerVectorSet final | |
///@name Access | ||
///@{ | ||
|
||
MutablePass GetMutablePass() | ||
{ | ||
return MutablePass(*this); | ||
} | ||
|
||
/** Gives a reference to underly normal container. */ | ||
TContainerType& GetContainer() | ||
{ | ||
|
@@ -1062,6 +1136,8 @@ class PointerVectorSet final | |
/// The maximum buffer size for data storage. | ||
size_type mMaxBufferSize; | ||
|
||
std::atomic<bool> mHasMutablePass = false; | ||
|
||
///@} | ||
///@name Private Operators | ||
///@{ | ||
|
@@ -1213,7 +1289,7 @@ class PointerVectorSet final | |
} | ||
|
||
///@} | ||
///@name Serialization | ||
///@name Friend classes | ||
///@{ | ||
|
||
/** | ||
|
@@ -1222,6 +1298,19 @@ class PointerVectorSet final | |
*/ | ||
friend class Serializer; | ||
|
||
/** | ||
* @class MutablePass | ||
* @brief A friend class responsible for giving a pass to mutate which may destroy the sorted state | ||
* of PointerVectorSet. All methods which relies on the sorted state will be frozen when this is | ||
* active, and they will be unfrozen and PointerVectorSet will be sorted at the destruction on the | ||
* MutablePass. | ||
*/ | ||
friend class MutablePass; | ||
|
||
///@} | ||
///@name Serialization | ||
///@{ | ||
|
||
/** | ||
* @brief Extract the object's state and uses the Serializer to store it. | ||
* @param rSerializer Serializer instance to be used for saving. | ||
|
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated