From ac317699d89849bed7498b8ee313151b2ffededb Mon Sep 17 00:00:00 2001 From: Mark Olesen Date: Mon, 6 May 2019 08:34:39 +0200 Subject: [PATCH] ENH: additional HashTable emplace/insert/set methods (#1286) - support move insert/set and emplace insertion. These adjustments can be used for improved memory efficiency, and allow hash tables of non-copyable objects (eg, std::unique_ptr). - extend special HashTable output treatment to include pointer-like objects such as autoPtr and unique_ptr. ENH: HashTable::at() method with checking. Fatal if entry does not exist. --- .../test/HashPtrTable/Test-HashPtrTable.C | 31 +++++-- .../test/HashTable2/Test-HashTable2.C | 22 +++++ .../test/HashTable4/Test-HashTable4.C | 12 +-- applications/test/ListOps2/Test-ListOps2.C | 10 ++- .../containers/HashTables/HashOps/HashOps.H | 8 +- .../containers/HashTables/HashSet/HashSet.H | 18 ++-- .../HashTables/HashTable/HashTable.C | 14 ++-- .../HashTables/HashTable/HashTable.H | 51 +++++++---- .../HashTables/HashTable/HashTableDetail.H | 45 +++++++--- .../HashTables/HashTable/HashTableI.H | 84 +++++++++++++++++-- .../HashTables/HashTable/HashTableIO.C | 2 +- 11 files changed, 227 insertions(+), 70 deletions(-) diff --git a/applications/test/HashPtrTable/Test-HashPtrTable.C b/applications/test/HashPtrTable/Test-HashPtrTable.C index da6ca717e8..19d6a261b7 100644 --- a/applications/test/HashPtrTable/Test-HashPtrTable.C +++ b/applications/test/HashPtrTable/Test-HashPtrTable.C @@ -2,7 +2,7 @@ ========= | \\ / F ield | OpenFOAM: The Open Source CFD Toolbox \\ / O peration | - \\ / A nd | Copyright (C) 2017-2018 OpenCFD Ltd. + \\ / A nd | Copyright (C) 2017-2019 OpenCFD Ltd. \\/ M anipulation | ------------------------------------------------------------------------------- | Copyright (C) 2011 OpenFOAM Foundation @@ -25,9 +25,9 @@ License Description - \*---------------------------------------------------------------------------*/ +#include #include #include "autoPtr.H" #include "HashPtrTable.H" @@ -56,14 +56,14 @@ void printTable(const HashPtrTable& table) Info<< ")" << endl; - // Values only, with for-range + // Iterate across values, with for-range Info<< "values ("; - for (auto val : table) + for (const auto& ptr : table) { Info<< ' '; - if (val) + if (ptr) { - Info<< *val; + Info<< *ptr; } else { @@ -86,8 +86,27 @@ int main() myTable.set("natlog", new double(2.718282)); myTable.insert("sqrt2", autoPtr::New(1.414214)); + HashTable, word, string::hash> myTable1; + + myTable1.set("abc", std::unique_ptr(new double(42.1))); + myTable1.set("pi", std::unique_ptr(new double(3.14159))); + myTable1.set("natlog", std::unique_ptr(new double(2.718282))); + + HashTable, word, string::hash> myTable2; + + myTable2.set("abc", autoPtr(new double(42.1))); + myTable2.set("pi", autoPtr(new double(3.14159))); + myTable2.set("natlog", autoPtr(new double(2.718282))); + myTable2.insert("sqrt2", autoPtr::New(1.414214)); + // Info<< myTable << endl; printTable(myTable); + Info<< myTable2 << nl; + + auto iter2 = myTable2.find("pi"); + + Info<<"PI: " << **iter2 << nl; + HashPtrTable copy(myTable); diff --git a/applications/test/HashTable2/Test-HashTable2.C b/applications/test/HashTable2/Test-HashTable2.C index 8e343a71d3..87ed286a8b 100644 --- a/applications/test/HashTable2/Test-HashTable2.C +++ b/applications/test/HashTable2/Test-HashTable2.C @@ -249,6 +249,28 @@ int main(int argc, char *argv[]) Info<< nl << "Ending scope" << nl; } + { + Info<< nl << "Table copy/move/emplace insertion" << nl; + + HashTable ltable1(0); + ltable1.insert("abc", identity(2)); + ltable1.insert("def", identity(3)); + ltable1.insert("ghi", identity(4)); + ltable1.emplace("jkl", 10, -35); + ltable1.emplace("mno"); + + labelList list1(identity(4, -4)); + + Info<<"move insert " << list1 << nl; + + ltable1.insert("pqr", std::move(list1)); + + Info<<"after insert " << list1 << nl; + + Info<< nl << "HashTable: " + << ltable1 << nl; + } + Info<< "\nEnd\n" << endl; return 0; diff --git a/applications/test/HashTable4/Test-HashTable4.C b/applications/test/HashTable4/Test-HashTable4.C index 343d89e07e..0a2655d2ce 100644 --- a/applications/test/HashTable4/Test-HashTable4.C +++ b/applications/test/HashTable4/Test-HashTable4.C @@ -145,15 +145,15 @@ int main(int argc, char *argv[]) const label nElem = 1000000; argList::noBanner(); - argList::addBoolOption("std", "use std::unordered_map or std::set"); - argList::addBoolOption("set", "test HashSet"); argList::addBoolOption("find", "test find"); + argList::addBoolOption("set", "test HashSet"); + argList::addBoolOption("std", "std::unordered_map or std::unordered_set"); argList args(argc, argv); - const bool optStd = args.found("std"); - const bool optSet = args.found("set"); const bool optFnd = args.found("find"); + const bool optSet = args.found("set"); + const bool optStd = args.found("std"); cpuTime timer; @@ -171,7 +171,7 @@ int main(int argc, char *argv[]) if (false) { - // verify that resizing around (0) doesn't fail + // Verify that resizing around (0) doesn't fail HashTable> map(32); printInfo(Info, map) << endl; @@ -243,7 +243,7 @@ int main(int argc, char *argv[]) #ifdef ORDERED Info<< "using stl::map" << endl; #else - Info<< "using stl::unordered_set" << endl; + Info<< "using stl::unordered_map" << endl; #endif for (label loopi = 0; loopi < nLoops; ++loopi) diff --git a/applications/test/ListOps2/Test-ListOps2.C b/applications/test/ListOps2/Test-ListOps2.C index f63e0e7e71..56904649cc 100644 --- a/applications/test/ListOps2/Test-ListOps2.C +++ b/applications/test/ListOps2/Test-ListOps2.C @@ -48,17 +48,21 @@ using namespace Foam; template class HashSorter; template -Ostream& operator<<(Ostream& os, const HashSorter& sorter); +Ostream& operator<< +( + Ostream& os, + const HashSorter& sorter +); template class HashSorter { - const HashTable& table; + const HashTable& table; public: - HashSorter(const HashTable& ht) + HashSorter(const HashTable& ht) : table(ht) {} diff --git a/src/OpenFOAM/containers/HashTables/HashOps/HashOps.H b/src/OpenFOAM/containers/HashTables/HashOps/HashOps.H index 9372a0ff64..f1939155e9 100644 --- a/src/OpenFOAM/containers/HashTables/HashOps/HashOps.H +++ b/src/OpenFOAM/containers/HashTables/HashOps/HashOps.H @@ -47,7 +47,7 @@ SourceFiles namespace Foam { -// Forward declarations +// Forward Declarations class bitSet; // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * // @@ -143,7 +143,11 @@ struct plusEqOp //- List of values from HashTable, optionally sorted. template -List values(const HashTable& tbl, const bool doSort=false) +List values +( + const HashTable& tbl, + const bool doSort=false +) { List output(tbl.size()); diff --git a/src/OpenFOAM/containers/HashTables/HashSet/HashSet.H b/src/OpenFOAM/containers/HashTables/HashSet/HashSet.H index 11bc4c55c8..4790fbcc9e 100644 --- a/src/OpenFOAM/containers/HashTables/HashSet/HashSet.H +++ b/src/OpenFOAM/containers/HashTables/HashSet/HashSet.H @@ -422,28 +422,28 @@ public: //- Combine entries from HashSets template -HashSet operator| +HashSet operator| ( - const HashSet& hash1, - const HashSet& hash2 + const HashSet& hash1, + const HashSet& hash2 ); //- Create a HashSet that only contains entries found in both HashSets template -HashSet operator& +HashSet operator& ( - const HashSet& hash1, - const HashSet& hash2 + const HashSet& hash1, + const HashSet& hash2 ); //- Create a HashSet that only contains unique entries (xor) template -HashSet operator^ +HashSet operator^ ( - const HashSet& hash1, - const HashSet& hash2 + const HashSet& hash1, + const HashSet& hash2 ); diff --git a/src/OpenFOAM/containers/HashTables/HashTable/HashTable.C b/src/OpenFOAM/containers/HashTables/HashTable/HashTable.C index 81e7e09a0b..6d65453275 100644 --- a/src/OpenFOAM/containers/HashTables/HashTable/HashTable.C +++ b/src/OpenFOAM/containers/HashTables/HashTable/HashTable.C @@ -300,11 +300,12 @@ Foam::label Foam::HashTable::countEntries template +template bool Foam::HashTable::setEntry ( + const bool overwrite, const Key& key, - const T& obj, - const bool overwrite + Args&&... args ) { if (!capacity_) @@ -330,9 +331,10 @@ bool Foam::HashTable::setEntry if (!curr) { // Not found, insert it at the head - table_[index] = new node_type(key, obj, table_[index]); - ++size_; + table_[index] = + new node_type(table_[index], key, std::forward(args)...); + ++size_; if (double(size_)/capacity_ > 0.8 && capacity_ < maxTableSize) { #ifdef FULLDEBUG @@ -360,7 +362,7 @@ bool Foam::HashTable::setEntry // or that it behaves the same as a copy construct. delete curr; - ep = new node_type(key, obj, ep); + ep = new node_type(ep, key, std::forward(args)...); // Replace current element - within list or insert at the head if (prev) @@ -838,7 +840,7 @@ bool Foam::HashTable::operator== { const const_iterator other(this->cfind(iter.key())); - if (!other.found() || other.val() != iter.val()) + if (!other.good() || other.val() != iter.val()) { return false; } diff --git a/src/OpenFOAM/containers/HashTables/HashTable/HashTable.H b/src/OpenFOAM/containers/HashTables/HashTable/HashTable.H index a0b91dfb4c..f973c85619 100644 --- a/src/OpenFOAM/containers/HashTables/HashTable/HashTable.H +++ b/src/OpenFOAM/containers/HashTables/HashTable/HashTable.H @@ -97,7 +97,7 @@ SourceFiles namespace Foam { -// Forward declarations +// Forward Declarations template class List; template class UList; @@ -155,7 +155,8 @@ class HashTable //- Assign a new hash-entry to a possibly already existing key. // \return True if the new entry was set. - bool setEntry(const Key& key, const T& obj, const bool overwrite); + template + bool setEntry(const bool overwrite, const Key& key, Args&&... args); public: @@ -257,6 +258,12 @@ public: //- Return true if the hash table is empty inline bool empty() const; + //- Find and return a hashed entry. FatalError if it does not exist. + inline T& at(const Key& key); + + //- Find and return a hashed entry. FatalError if it does not exist. + inline const T& at(const Key& key) const; + //- Return true if hashed entry is found in table inline bool found(const Key& key) const; @@ -361,16 +368,27 @@ public: // Edit - //- Insert a new entry, not overwriting existing entries. - // \return True if the entry inserted, which means that it did - // not previously exist in the table. + //- Emplace insert a new entry, not overwriting existing entries. + // \return True if the entry did not previously exist in the table. + template + inline bool emplace(const Key& key, Args&&... args); + + //- Copy insert a new entry, not overwriting existing entries. + // \return True if the entry did not previously exist in the table. inline bool insert(const Key& key, const T& obj); - //- Assign a new entry, overwriting existing entries. - // + //- Move insert a new entry, not overwriting existing entries. + // \return True if the entry did not previously exist in the table. + inline bool insert(const Key& key, T&& obj); + + //- Copy assign a new entry, overwriting existing entries. // \return True, since it always overwrites any entries. inline bool set(const Key& key, const T& obj); + //- Move assign a new entry, overwriting existing entries. + // \return True, since it always overwrites any entries. + inline bool set(const Key& key, T&& obj); + //- Erase an entry specified by given iterator // This invalidates the iterator until the next ++ operation. // @@ -513,20 +531,20 @@ public: inline T& operator()(const Key& key, const T& deflt); //- Copy assign - void operator=(const HashTable& rhs); + void operator=(const this_type& rhs); //- Copy assign from an initializer list void operator=(std::initializer_list> rhs); //- Move assign - void operator=(HashTable&& rhs); + void operator=(this_type&& rhs); //- Equality. Tables are equal if all keys and values are equal, //- independent of order or underlying storage size. - bool operator==(const HashTable& rhs) const; + bool operator==(const this_type& rhs) const; //- The opposite of the equality operation. - bool operator!=(const HashTable& rhs) const; + bool operator!=(const this_type& rhs) const; //- Add entries into this HashTable this_type& operator+=(const this_type& rhs); @@ -584,8 +602,7 @@ protected: // This can be used directly instead of comparing to end() inline bool good() const; - //- True if iterator points to an entry - // This can be used directly instead of comparing to end() + //- True if iterator points to an entry - same as good() inline bool found() const; //- The key associated with the iterator @@ -703,7 +720,7 @@ public: {} - // Member functions/operators + // Member Functions/Operators //- Non-const access to referenced object (value) using Iterator::val; @@ -719,7 +736,7 @@ public: template typename std::enable_if < - std::is_pointer::value, + Detail::isPointer::value, T >::type operator->() const { return this->val(); } @@ -773,7 +790,7 @@ public: {} - // Member functions/operators + // Member Functions/Operators //- Const access to referenced value using Iterator::val; @@ -789,7 +806,7 @@ public: template typename std::enable_if < - std::is_pointer::value, + Detail::isPointer::value, const T >::type operator->() const { return this->val(); } diff --git a/src/OpenFOAM/containers/HashTables/HashTable/HashTableDetail.H b/src/OpenFOAM/containers/HashTables/HashTable/HashTableDetail.H index 5d3baece87..84d41e678d 100644 --- a/src/OpenFOAM/containers/HashTables/HashTable/HashTableDetail.H +++ b/src/OpenFOAM/containers/HashTables/HashTable/HashTableDetail.H @@ -36,6 +36,7 @@ SourceFiles #define HashTableDetail_H #include "zero.H" +#include #include #include @@ -44,14 +45,32 @@ SourceFiles namespace Foam { -// Forward declarations +// Forward Declarations class Ostream; +template class autoPtr; namespace Detail { /*---------------------------------------------------------------------------*\ - Class Detail::HashTablePair Declaration + Class isPointer Declaration +\*---------------------------------------------------------------------------*/ + +//- Test for pointer-like behaviour +template +struct isPointer : public std::is_pointer {}; + +//- An autoPtr is pointer-like +template +struct isPointer> : public std::true_type {}; + +//- A unique_ptr is pointer-like +template +struct isPointer> : public std::true_type {}; + + +/*---------------------------------------------------------------------------*\ + Class HashTablePair Declaration \*---------------------------------------------------------------------------*/ //- Internal storage type for HashTable entries @@ -97,16 +116,17 @@ struct HashTablePair void operator=(const HashTablePair&) = delete; - //- Construct from key, value, next pointer + //- Construct from next pointer, key, contents + template HashTablePair ( + HashTablePair* next, const key_type& key, - const mapped_type& val, - HashTablePair* next + Args&&... args ) : key_(key), - val_(val), + val_(std::forward(args)...), next_(next) {} @@ -131,7 +151,7 @@ struct HashTablePair //- Write (key, val) pair - for pointer types template - typename std::enable_if::value, void>::type + typename std::enable_if::value, void>::type print(Ostream& os) const { os << key_; @@ -144,7 +164,7 @@ struct HashTablePair //- Write (key, val) pair - for non-pointer types template - typename std::enable_if::value, void>::type + typename std::enable_if::value, void>::type print(Ostream& os) const { os << key_ << ' ' << val_; @@ -153,7 +173,7 @@ struct HashTablePair /*---------------------------------------------------------------------------*\ - Class Detail::HashTableSingle Declaration + Class HashTableSingle Declaration \*---------------------------------------------------------------------------*/ //- Internal storage type for HashSet entries @@ -196,12 +216,13 @@ struct HashTableSingle void operator=(const HashTableSingle&) = delete; - //- Construct from key, (ununsed) value, next pointer + //- Construct from next pointer, key, (ununsed) contents + template HashTableSingle ( + HashTableSingle* next, const key_type& key, - const mapped_type&, - HashTableSingle* next + Args&&... ) : key_(key), diff --git a/src/OpenFOAM/containers/HashTables/HashTable/HashTableI.H b/src/OpenFOAM/containers/HashTables/HashTable/HashTableI.H index fba4679694..9342a71424 100644 --- a/src/OpenFOAM/containers/HashTables/HashTable/HashTableI.H +++ b/src/OpenFOAM/containers/HashTables/HashTable/HashTableI.H @@ -61,12 +61,46 @@ inline bool Foam::HashTable::empty() const } +template +inline T& Foam::HashTable::at(const Key& key) +{ + const iterator iter(this->find(key)); + + if (!iter.good()) + { + FatalErrorInFunction + << key << " not found in table. Valid entries: " + << toc() + << exit(FatalError); + } + + return iter.val(); +} + + +template +inline const T& Foam::HashTable::at(const Key& key) const +{ + const const_iterator iter(this->cfind(key)); + + if (!iter.good()) + { + FatalErrorInFunction + << key << " not found in table. Valid entries: " + << toc() + << exit(FatalError); + } + + return iter.val(); +} + + template inline bool Foam::HashTable::found(const Key& key) const { if (size_) { - return Iterator(this, key).found(); + return Iterator(this, key).good(); } return false; @@ -116,6 +150,18 @@ Foam::HashTable::cfind } +template +template +inline bool Foam::HashTable::emplace +( + const Key& key, + Args&&... args +) +{ + return this->setEntry(false, key, std::forward(args)...); +} + + template inline bool Foam::HashTable::insert ( @@ -123,7 +169,18 @@ inline bool Foam::HashTable::insert const T& val ) { - return this->setEntry(key, val, false); // No overwrite + return this->setEntry(false, key, val); +} + + +template +inline bool Foam::HashTable::insert +( + const Key& key, + T&& val +) +{ + return this->setEntry(false, key, std::forward(val)); } @@ -134,7 +191,18 @@ inline bool Foam::HashTable::set const T& val ) { - return this->setEntry(key, val, true); // Overwrite + return this->setEntry(true, key, val); // Overwrite +} + + +template +inline bool Foam::HashTable::set +( + const Key& key, + T&& val +) +{ + return this->setEntry(true, key, std::forward(val)); // Overwrite } @@ -146,7 +214,7 @@ inline const T& Foam::HashTable::lookup ) const { const const_iterator iter(this->cfind(key)); - return iter.found() ? iter.val() : deflt; + return iter.good() ? iter.val() : deflt; } @@ -157,7 +225,7 @@ inline T& Foam::HashTable::operator[](const Key& key) { const iterator iter(this->find(key)); - if (!iter.found()) + if (!iter.good()) { FatalErrorInFunction << key << " not found in table. Valid entries: " @@ -174,7 +242,7 @@ inline const T& Foam::HashTable::operator[](const Key& key) const { const const_iterator iter(this->cfind(key)); - if (!iter.found()) + if (!iter.good()) { FatalErrorInFunction << key << " not found in table. Valid entries: " @@ -191,7 +259,7 @@ inline T& Foam::HashTable::operator()(const Key& key) { const iterator iter(this->find(key)); - if (iter.found()) + if (iter.good()) { return iter.val(); } @@ -210,7 +278,7 @@ inline T& Foam::HashTable::operator() { const iterator iter(this->find(key)); - if (iter.found()) + if (iter.good()) { return iter.val(); } diff --git a/src/OpenFOAM/containers/HashTables/HashTable/HashTableIO.C b/src/OpenFOAM/containers/HashTables/HashTable/HashTableIO.C index ce5dca1ab4..ec3922f691 100644 --- a/src/OpenFOAM/containers/HashTables/HashTable/HashTableIO.C +++ b/src/OpenFOAM/containers/HashTables/HashTable/HashTableIO.C @@ -39,7 +39,7 @@ bool Foam::HashTable::addEntry(Istream& is, const bool overwrite) is >> key >> val; - const bool ok = this->setEntry(key, val, overwrite); + const bool ok = this->setEntry(overwrite, key, val); is.fatalCheck (