From 35d348c00d1356c08ee18d98eb227518ac4e87dd Mon Sep 17 00:00:00 2001 From: Mark Olesen Date: Wed, 21 Mar 2018 15:30:14 +0100 Subject: [PATCH] BUG: guard against potential wasted memory in DynamicField - specialize transfer and swap to ensure allocated capacity isn't forgotten. --- .../fields/Fields/DynamicField/DynamicField.C | 5 +- .../fields/Fields/DynamicField/DynamicField.H | 175 +++++++------ .../Fields/DynamicField/DynamicFieldI.H | 237 ++++++++++++++---- src/OpenFOAM/fields/Fields/Field/FieldI.H | 1 - 4 files changed, 296 insertions(+), 122 deletions(-) diff --git a/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.C b/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.C index a18d76fc9e..8598b3c1f6 100644 --- a/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.C +++ b/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.C @@ -39,10 +39,7 @@ template Foam::tmp> Foam::DynamicField::clone() const { - return tmp> - ( - new DynamicField(*this) - ); + return tmp>::New(*this); } diff --git a/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.H b/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.H index 80e7455006..18559ad7b6 100644 --- a/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.H +++ b/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.H @@ -51,20 +51,20 @@ template class DynamicField; template Ostream& operator<< ( - Ostream&, - const DynamicField& + Ostream& os, + const DynamicField& fld ); template Istream& operator>> ( - Istream&, - DynamicField& + Istream& is, + DynamicField& fld ); /*---------------------------------------------------------------------------*\ - Class DynamicField Declaration + Class DynamicField Declaration \*---------------------------------------------------------------------------*/ template @@ -84,7 +84,7 @@ class DynamicField //- Copy assignment from another list template - inline void assignDynField(const ListType& lst); + inline void assignDynField(const ListType& list); public: @@ -102,15 +102,25 @@ public: //- Construct null inline constexpr DynamicField() noexcept; - //- Construct with given capacity. + //- Construct empty field with given reserve size. explicit inline DynamicField(const label len); + //- Construct given size and initial value + inline DynamicField(const label len, const T& val); + + //- Construct given size and initial value of zero + inline DynamicField(const label len, const zero); + //- Copy construct - inline DynamicField(const DynamicField& lst); + inline DynamicField(const DynamicField& list); + + //- Copy construct with different sizing parameters + template + inline DynamicField(const DynamicField& list); //- Copy construct from UList. Size set to UList size. // Also constructs from DynamicField with different sizing parameters. - explicit inline DynamicField(const UList& lst); + explicit inline DynamicField(const UList& list); //- Copy construct from UIndirectList explicit inline DynamicField(const UIndirectList& list); @@ -118,9 +128,13 @@ public: //- Move construct from List contents explicit inline DynamicField(List&& content); - //- Move construct from Field contents + //- Move construct from dynamic Field contents inline DynamicField(DynamicField&& content); + //- Move construct with different sizing parameters + template + inline DynamicField(DynamicField&& content); + //- Construct by 1 to 1 mapping from the given field inline DynamicField ( @@ -152,87 +166,108 @@ public: // Member Functions - // Access + // Access - //- Size of the underlying storage. - inline label capacity() const; + //- Size of the underlying storage. + inline label capacity() const; - // Edit + // Edit - //- Alter the size of the underlying storage. - // The addressed size will be truncated if needed to fit, but will - // remain otherwise untouched. - // Use this or reserve() in combination with append(). - inline void setCapacity(const label nElem); + //- Alter the size of the underlying storage. + // The addressed size will be truncated if needed to fit, but will + // remain otherwise untouched. + // Use this or reserve() in combination with append(). + inline void setCapacity(const label nElem); - //- Alter the addressed list size. - // New space will be allocated if required. - // Use this to resize the list prior to using the operator[] for - // setting values (as per List usage). - inline void setSize(const label nElem); + //- Alter the addressed list size. + // New space will be allocated if required. + // Use this to resize the list prior to using the operator[] for + // setting values (as per List usage). + inline void setSize(const label nElem); - //- Alter the addressed list size and fill new space with a - // constant. - inline void setSize(const label nElem, const T& val); + //- Alter the addressed list size and fill new space with a constant. + inline void setSize(const label nElem, const T& val); - //- Alter the addressed list size. - // New space will be allocated if required. - // Use this to resize the list prior to using the operator[] for - // setting values (as per List usage). - inline void resize(const label nElem); + //- Alter the addressed list size. + // New space will be allocated if required. + // Use this to resize the list prior to using the operator[] for + // setting values (as per List usage). + inline void resize(const label nElem); - //- Alter the addressed list size and fill new space with a - // constant. - inline void resize(const label nElem, const T& val); + //- Alter the addressed list size and fill new space with a + // constant. + inline void resize(const label nElem, const T& val); - //- Reserve allocation space for at least this size. - // Never shrinks the allocated size, use setCapacity() for that. - inline void reserve(const label nElem); + //- Reserve allocation space for at least this size. + // Never shrinks the allocated size, use setCapacity() for that. + inline void reserve(const label nElem); - //- Clear the addressed list, i.e. set the size to zero. - // Allocated size does not change - inline void clear(); + //- Clear the addressed list, i.e. set the size to zero. + // Allocated size does not change + inline void clear(); - //- Clear the list and delete storage. - inline void clearStorage(); + //- Clear the list and delete storage. + inline void clearStorage(); - //- Expand the addressable size to fit the allocated capacity. - // Returns the previous addressable size. - inline label expandStorage(); + //- Expand the addressable size to fit the allocated capacity. + // Returns the previous addressable size. + inline label expandStorage(); - //- Shrink the allocated space to the number of elements used. - // Returns a reference to the DynamicField. - inline DynamicField& shrink(); + //- Shrink the allocated space to the number of elements used. + // Returns a reference to the DynamicField. + inline DynamicField& shrink(); + + //- Swap content with any sized DynamicField + template + inline void swap(DynamicField& list); + + //- Transfer the parameter contents into this + inline void transfer(List& list); + + //- Transfer the parameter contents into this + template + inline void transfer(DynamicList& list); + + //- Transfer the parameter contents into this + template + inline void transfer(DynamicField& list); - // Member Operators + //- Append an element at the end of the list + inline DynamicField& + append(const T& val); - //- Append an element at the end of the list - inline DynamicField& - append(const T& val); + //- Append a List at the end of this list + inline DynamicField& + append(const UList& list); - //- Append a List at the end of this list - inline DynamicField& - append(const UList& lst); + //- Remove and return the top element + inline T remove(); - //- Remove and return the top element - inline T remove(); - //- Return non-const access to an element, resizing list if - // necessary - inline T& operator()(const label i); + // Member Operators - //- Assignment of all addressed entries to the given value - inline void operator=(const T& val); + //- Return non-const access to an element, resizing list if necessary + inline T& operator()(const label i); - //- Assignment to DynamicField - inline void operator= - ( - const DynamicField& lst - ); + //- Assign addressed entries to the given value + inline void operator=(const T& val); - //- Assignment to UList - inline void operator=(const UList& lst); + //- Copy assignment + inline void operator=(const UList& list); + + //- Copy assignment + inline void operator=(const DynamicField& list); + + //- Move assignment + inline void operator=(List&& list); + + //- Move assignment + inline void operator=(DynamicField&& list); + + //- Move assignment + template + inline void operator=(DynamicField&& list); }; diff --git a/src/OpenFOAM/fields/Fields/DynamicField/DynamicFieldI.H b/src/OpenFOAM/fields/Fields/DynamicField/DynamicFieldI.H index 1f7b5c479f..6fdb18241a 100644 --- a/src/OpenFOAM/fields/Fields/DynamicField/DynamicFieldI.H +++ b/src/OpenFOAM/fields/Fields/DynamicField/DynamicFieldI.H @@ -29,23 +29,23 @@ template template inline void Foam::DynamicField::assignDynField ( - const ListType& lst + const ListType& list ) { - const label newSize = lst.size(); + const label newSize = list.size(); if (capacity_ >= newSize) { // Can copy w/o reallocating - adjust addressable size accordingly. - Field::size(lst.size()); - Field::operator=(lst); + Field::size(list.size()); + Field::operator=(list); } else { // Ensure list size consistency prior to copying. Field::size(capacity_); - Field::operator=(lst); + Field::operator=(list); capacity_ = Field::size(); } } @@ -62,15 +62,11 @@ inline constexpr Foam::DynamicField::DynamicField() noexcept template -inline Foam::DynamicField::DynamicField -( - const label len -) +inline Foam::DynamicField::DynamicField(const label len) : Field(len), capacity_(Field::size()) { - // We could also enforce sizing granularity Field::size(0); } @@ -78,21 +74,11 @@ inline Foam::DynamicField::DynamicField template inline Foam::DynamicField::DynamicField ( - const DynamicField& lst + const label len, + const T& val ) : - Field(lst), - capacity_(lst.capacity()) -{} - - -template -inline Foam::DynamicField::DynamicField -( - const UList& lst -) -: - Field(lst), + Field(len, val), capacity_(Field::size()) {} @@ -100,10 +86,56 @@ inline Foam::DynamicField::DynamicField template inline Foam::DynamicField::DynamicField ( - const UIndirectList& lst + const label len, + const zero ) : - Field(lst), + Field(len, Zero), + capacity_(Field::size()) +{} + + +template +inline Foam::DynamicField::DynamicField +( + const DynamicField& list +) +: + Field(list), + capacity_(Field::size()) +{} + + +template +template +inline Foam::DynamicField::DynamicField +( + const DynamicField& list +) +: + Field(list), + capacity_(Field::size()) +{} + + +template +inline Foam::DynamicField::DynamicField +( + const UList& list +) +: + Field(list), + capacity_(Field::size()) +{} + + +template +inline Foam::DynamicField::DynamicField +( + const UIndirectList& list +) +: + Field(list), capacity_(Field::size()) {} @@ -125,10 +157,24 @@ inline Foam::DynamicField::DynamicField DynamicField&& content ) : - Field(std::move(content)), - capacity_(Field::size()) + Field(), + capacity_(0) { - content.clear(); + transfer(content); +} + + +template +template +inline Foam::DynamicField::DynamicField +( + DynamicField&& content +) +: + Field(), + capacity_(0) +{ + transfer(content); } @@ -173,8 +219,7 @@ inline Foam::DynamicField::DynamicField // * * * * * * * * * * * * * * * Member Functions * * * * * * * * * * * * * // template -inline Foam::label Foam::DynamicField::capacity() -const +inline Foam::label Foam::DynamicField::capacity() const { return capacity_; } @@ -343,6 +388,73 @@ Foam::DynamicField::shrink() } +template +template +inline void Foam::DynamicField::swap +( + DynamicField& lst +) +{ + DynamicList& cur = *this; + + // Make addressable size identical to the allocated capacity + const label oldSize1 = cur.expandStorage(); + const label oldSize2 = lst.expandStorage(); + + // Swap storage + Field::swap(lst); + + // Match capacity to the underlying allocated list size + cur.setCapacity(cur.size()); + lst.setCapacity(lst.size()); + + // Set addressable size + cur.setSize(oldSize2); + lst.setSize(oldSize1); +} + + +template +inline void Foam::DynamicField::transfer(List& list) +{ + // Take over storage, clear addressing for list. + capacity_ = list.size(); + Field::transfer(list); +} + + +template +template +inline void Foam::DynamicField::transfer +( + DynamicList& list +) +{ + // Take over storage as-is (without shrink, without using SizeMin) + // clear addressing and storage for old list. + capacity_ = list.capacity(); + + Field::transfer(static_cast&>(list)); + list.clearStorage(); // Ensure capacity=0 +} + + +template +template +inline void Foam::DynamicField::transfer +( + DynamicField& list +) +{ + // Take over storage as-is (without shrink, without using SizeMin) + // clear addressing and storage for old list. + capacity_ = list.capacity(); + + Field::transfer(static_cast&>(list)); + list.clearStorage(); // Ensure capacity=0 +} + + template inline Foam::DynamicField& Foam::DynamicField::append @@ -353,7 +465,7 @@ Foam::DynamicField::append const label idx = List::size(); setSize(idx + 1); - this->operator[](idx) = val; + this->operator[](idx) = val; // copy element return *this; } @@ -362,21 +474,21 @@ template inline Foam::DynamicField& Foam::DynamicField::append ( - const UList& lst + const UList& list ) { - if (this == &lst) + if (this == &list) { FatalErrorInFunction << "attempted appending to self" << abort(FatalError); } - label nextFree = List::size(); - setSize(nextFree + lst.size()); + label idx = List::size(); + setSize(idx + list.size()); - forAll(lst, i) + for (const T& val : list) { - this->operator[](nextFree++) = lst[i]; + this->operator[](idx++) = val; // copy element } return *this; } @@ -432,26 +544,57 @@ inline void Foam::DynamicField::operator= template inline void Foam::DynamicField::operator= ( - const DynamicField& lst + const UList& list ) { - if (this == &lst) - { - FatalErrorInFunction - << "Attempted assignment to self" << abort(FatalError); - } - - assignDynField(lst); + assignDynField(list); } template inline void Foam::DynamicField::operator= ( - const UList& lst + const DynamicField& list ) { - assignDynField(lst); + if (this == &list) + { + FatalErrorInFunction + << "Attempted assignment to self" << abort(FatalError); + } + + assignDynField(list); +} + + +template +inline void Foam::DynamicField::operator= +( + List&& list +) +{ + transfer(list); +} + + +template +inline void Foam::DynamicField::operator= +( + DynamicField&& list +) +{ + transfer(list); +} + + +template +template +inline void Foam::DynamicField::operator= +( + DynamicField&& list +) +{ + transfer(list); } diff --git a/src/OpenFOAM/fields/Fields/Field/FieldI.H b/src/OpenFOAM/fields/Fields/Field/FieldI.H index 10101a2b2f..987d647c6a 100644 --- a/src/OpenFOAM/fields/Fields/Field/FieldI.H +++ b/src/OpenFOAM/fields/Fields/Field/FieldI.H @@ -186,5 +186,4 @@ inline void Foam::Field::operator=(const zero) } - // ************************************************************************* //