Commit 7fdd6bb4 authored by Mark OLESEN's avatar Mark OLESEN
Browse files

ENH: partial reorganization of HashTable internals (#1160)

- relocate the pair_entry (HashTable) and unary_entry (HashSet) into
  the Detail namespace and add output handling.

  The output handling at this level removes the reliance on zero::null
  output (HashSet) and allows direct support of pointers.
  This means that the following now works

      HashTable<T*> tbl;
      os << tbl;

  It also means that we don't need to overload operator<< for
  HashPtrTable anymore.

- avoid delete/new when calling HashSet::set(). If the entry already
  exists there is no reason to remove it and add another one with the
  same content.

STYLE: HashTable iterators now have a val() method

- identical to the object() iterator method, but shorter to type.
parent 0728c7e8
......@@ -2,7 +2,7 @@
========= |
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2018 OpenCFD Ltd.
\\ / A nd | Copyright (C) 2018-2019 OpenCFD Ltd.
\\/ M anipulation |
-------------------------------------------------------------------------------
License
......@@ -151,7 +151,7 @@ List<T> values(const HashTable<T, Key, Hash>& tbl, const bool doSort=false)
forAllConstIters(tbl, iter)
{
output[i] = iter.object();
output[i] = iter.val();
++i;
}
......
......@@ -3,7 +3,7 @@
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2011-2016 OpenFOAM Foundation
\\/ M anipulation | Copyright (C) 2017-2018 OpenCFD Ltd.
\\/ M anipulation | Copyright (C) 2017-2019 OpenCFD Ltd.
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
......@@ -38,7 +38,7 @@ Foam::HashPtrTable<T, Key, Hash>::HashPtrTable
{
for (const_iterator iter = ht.begin(); iter != ht.end(); ++iter)
{
const T* ptr = iter.object();
const T* ptr = iter.val();
if (ptr)
{
this->set(iter.key(), new T(*ptr));
......@@ -77,7 +77,7 @@ Foam::autoPtr<T> Foam::HashPtrTable<T, Key, Hash>::remove(iterator& iter)
{
if (iter.good())
{
autoPtr<T> aptr(iter.object());
autoPtr<T> aptr(iter.val());
this->parent_type::erase(iter);
return aptr;
}
......@@ -99,7 +99,7 @@ bool Foam::HashPtrTable<T, Key, Hash>::erase(iterator& iter)
{
if (iter.good())
{
T* ptr = iter.object();
T* ptr = iter.val();
if (this->parent_type::erase(iter))
{
......@@ -129,7 +129,7 @@ void Foam::HashPtrTable<T, Key, Hash>::clear()
{
for (iterator iter = this->begin(); iter != this->end(); ++iter)
{
delete iter.object();
delete iter.val();
}
this->parent_type::clear();
......@@ -155,7 +155,7 @@ void Foam::HashPtrTable<T, Key, Hash>::operator=
for (const_iterator iter = rhs.begin(); iter != rhs.end(); ++iter)
{
const T* ptr = iter.object();
const T* ptr = iter.val();
if (ptr)
{
this->set(iter.key(), new T(*ptr));
......
......@@ -54,9 +54,6 @@ template<class T, class Key, class Hash> class HashPtrTable;
template<class T, class Key, class Hash>
Istream& operator>>(Istream& is, HashPtrTable<T, Key, Hash>& tbl);
template<class T, class Key, class Hash>
Ostream& operator<<(Ostream& os, const HashPtrTable<T, Key, Hash>& tbl);
/*---------------------------------------------------------------------------*\
Class HashPtrTable Declaration
......@@ -171,12 +168,6 @@ public:
HashPtrTable<T, Key, Hash>& tbl
);
friend Ostream& operator<< <T, Key, Hash>
(
Ostream& os,
const HashPtrTable<T, Key, Hash>& tbl
);
// Housekeeping
......
......@@ -3,7 +3,7 @@
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2011-2015 OpenFOAM Foundation
\\/ M anipulation | Copyright (C) 2017-2018 OpenCFD Ltd.
\\/ M anipulation | Copyright (C) 2017-2019 OpenCFD Ltd.
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
......@@ -153,7 +153,7 @@ void Foam::HashPtrTable<T, Key, Hash>::write(Ostream& os) const
{
for (const_iterator iter = this->cbegin(); iter != this->cend(); ++iter)
{
const T* ptr = iter.object();
const T* ptr = iter.val();
if (ptr)
{
ptr->write(os);
......@@ -198,43 +198,4 @@ Foam::Istream& Foam::operator>>(Istream& is, HashPtrTable<T, Key, Hash>& tbl)
}
template<class T, class Key, class Hash>
Foam::Ostream& Foam::operator<<
(
Ostream& os,
const HashPtrTable<T, Key, Hash>& tbl
)
{
const label len = tbl.size();
if (len)
{
// Size and start list delimiter
os << nl << len << nl << token::BEGIN_LIST << nl;
// Contents
for (auto iter = tbl.cbegin(); iter != tbl.cend(); ++iter)
{
const T* ptr = iter.object();
os << iter.key();
if (ptr)
{
os << token::SPACE << *ptr;
}
os << nl;
}
os << token::END_LIST; // End list delimiter
}
else
{
// Empty hash table
os << len << token::BEGIN_LIST << token::END_LIST;
}
os.check(FUNCTION_NAME);
return os;
}
// ************************************************************************* //
......@@ -3,7 +3,7 @@
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2011-2016 OpenFOAM Foundation
\\/ M anipulation | Copyright (C) 2017-2018 OpenCFD Ltd.
\\/ M anipulation | Copyright (C) 2017-2019 OpenCFD Ltd.
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
......@@ -65,7 +65,7 @@ Foam::HashTable<T, Key, Hash>::HashTable(const HashTable<T, Key, Hash>& ht)
{
for (const_iterator iter = ht.cbegin(); iter != ht.cend(); ++iter)
{
insert(iter.key(), iter.object());
insert(iter.key(), iter.val());
}
}
......@@ -87,12 +87,12 @@ Foam::HashTable<T, Key, Hash>::HashTable(HashTable<T, Key, Hash>&& rhs)
template<class T, class Key, class Hash>
Foam::HashTable<T, Key, Hash>::HashTable
(
std::initializer_list<std::pair<Key, T>> lst
std::initializer_list<std::pair<Key, T>> list
)
:
HashTable<T, Key, Hash>(2*lst.size())
HashTable<T, Key, Hash>(2*list.size())
{
for (const auto& keyval : lst)
for (const auto& keyval : list)
{
insert(keyval.first, keyval.second);
}
......@@ -172,7 +172,7 @@ Foam::List<Key> Foam::HashTable<T, Key, Hash>::tocKeys
}
}
list.setSize(count);
list.resize(count);
Foam::sort(list);
return list;
......@@ -192,13 +192,13 @@ Foam::List<Key> Foam::HashTable<T, Key, Hash>::tocValues
for (const_iterator iter = cbegin(); iter != cend(); ++iter)
{
if ((pred(iter.object()) ? !invert : invert))
if ((pred(iter.val()) ? !invert : invert))
{
list[count++] = iter.key();
}
}
list.setSize(count);
list.resize(count);
Foam::sort(list);
return list;
......@@ -218,13 +218,13 @@ Foam::List<Key> Foam::HashTable<T, Key, Hash>::tocEntries
for (const_iterator iter = cbegin(); iter != cend(); ++iter)
{
if ((pred(iter.key(), iter.object()) ? !invert : invert))
if ((pred(iter.key(), iter.val()) ? !invert : invert))
{
list[count++] = iter.key();
}
}
list.setSize(count);
list.resize(count);
Foam::sort(list);
return list;
......@@ -265,7 +265,7 @@ Foam::label Foam::HashTable<T, Key, Hash>::countValues
for (const_iterator iter = cbegin(); iter != cend(); ++iter)
{
if ((pred(iter.object()) ? !invert : invert))
if ((pred(iter.val()) ? !invert : invert))
{
++count;
}
......@@ -287,7 +287,7 @@ Foam::label Foam::HashTable<T, Key, Hash>::countEntries
for (const_iterator iter = cbegin(); iter != cend(); ++iter)
{
if ((pred(iter.key(), iter.object()) ? !invert : invert))
if ((pred(iter.key(), iter.val()) ? !invert : invert))
{
++count;
}
......@@ -334,10 +334,7 @@ bool Foam::HashTable<T, Key, Hash>::setEntry
if (double(size_)/capacity_ > 0.8 && capacity_ < maxTableSize)
{
#ifdef FULLDEBUG
if (debug)
{
InfoInFunction << "Doubling table size\n";
}
DebugInFunction << "Doubling table size\n";
#endif
resize(2*capacity_);
......@@ -347,6 +344,13 @@ bool Foam::HashTable<T, Key, Hash>::setEntry
{
// Overwrite current entry (Perl convention).
// Can skip if the value is not stored anyhow (Eg, HashSet)
// - this avoids a useless delete/new
if (!node_type::stores_value())
{
return true;
}
node_type* ep = curr->next_; // next in the linked list
// In some cases the delete/new could be avoided in favour of move
......@@ -370,11 +374,7 @@ bool Foam::HashTable<T, Key, Hash>::setEntry
{
// Do not overwrite existing entry (STL 'insert' convention)
#ifdef FULLDEBUG
if (debug)
{
InfoInFunction
<< "Cannot insert " << key << " already in hash table\n";
}
DebugInFunction << "Not inserting " << key << ": already in table\n";
#endif
return false;
}
......@@ -553,10 +553,7 @@ void Foam::HashTable<T, Key, Hash>::resize(const label sz)
if (newCapacity == oldCapacity)
{
#ifdef FULLDEBUG
if (debug)
{
InfoInFunction << "New table size == old table size\n";
}
DebugInFunction << "New table size == old table size\n";
#endif
return;
......@@ -567,8 +564,7 @@ void Foam::HashTable<T, Key, Hash>::resize(const label sz)
if (size_)
{
WarningInFunction
<< "HashTable contains " << size_ << " cannot resize(0)"
<< endl;
<< "HashTable contains " << size_ << " cannot resize(0)" << nl;
}
else
{
......@@ -711,7 +707,7 @@ Foam::label Foam::HashTable<T, Key, Hash>::filterValues
// Matches? either prune (pruning) or keep (!pruning)
if
(
(pred(iter.object()) ? pruning : !pruning)
(pred(iter.val()) ? pruning : !pruning)
&& erase(iter)
)
{
......@@ -738,7 +734,7 @@ Foam::label Foam::HashTable<T, Key, Hash>::filterEntries
// Matches? either prune (pruning) or keep (!pruning)
if
(
(pred(iter.key(), iter.object()) ? pruning : !pruning)
(pred(iter.key(), iter.val()) ? pruning : !pruning)
&& erase(iter)
)
{
......@@ -778,7 +774,7 @@ void Foam::HashTable<T, Key, Hash>::operator=
for (const_iterator iter = rhs.cbegin(); iter != rhs.cend(); ++iter)
{
insert(iter.key(), iter.object());
insert(iter.key(), iter.val());
}
}
......@@ -840,7 +836,7 @@ bool Foam::HashTable<T, Key, Hash>::operator==
{
const const_iterator other(this->cfind(iter.key()));
if (!other.found() || other.object() != iter.object())
if (!other.found() || other.val() != iter.val())
{
return false;
}
......@@ -873,7 +869,7 @@ Foam::HashTable<T, Key, Hash>& Foam::HashTable<T, Key, Hash>::operator+=
{
for (const_iterator iter = rhs.cbegin(); iter != rhs.cend(); ++iter)
{
insert(iter.key(), iter.object());
insert(iter.key(), iter.val());
}
}
else
......
......@@ -46,8 +46,7 @@ Description
Note
For historical reasons, dereferencing the table iterator
(eg, \a *iter) returns a reference to the stored object value
value rather than the stored key/object value like std::unordered_map
does.
rather than the stored key/val pair like std::unordered_map does.
The HashTable iterator:
\code
......@@ -55,7 +54,7 @@ Note
{
Info<< "val:" << *iter << nl
<< "key:" << iter.key() << nl;
<< "val:" << iter.object() << nl;
<< "val:" << iter.val() << nl;
}
\endcode
whereas for the \c std::unordered_map iterator:
......@@ -84,6 +83,7 @@ SourceFiles
#include "word.H"
#include "zero.H"
#include "Hash.H"
#include "HashTableDetail.H"
#include "HashTableCore.H"
#include <initializer_list>
......@@ -103,7 +103,7 @@ template<class T, unsigned Size> class FixedList;
template<class T, class Key, class Hash> class HashTable;
template<class T, class Key, class Hash>
Istream& operator>>(Istream& is, HashTable<T, Key, Hash>& L);
Istream& operator>>(Istream& is, HashTable<T, Key, Hash>& tbl);
template<class T, class Key, class Hash>
Ostream& operator<<(Ostream& os, const HashTable<T, Key, Hash>& tbl);
......@@ -118,118 +118,14 @@ class HashTable
:
public HashTableCore
{
// Private types for table entries
// Types
//- Structure with a (K,V) tuple and a linked-list for collisions
// Could store key/object as std::pair, but no particular advantage
// unless the iterator dereference type changes.
struct pair_entry
{
//- Type of key
typedef Key key_type;
//- Object content type
typedef T mapped_type;
//- The lookup key
key_type key_;
//- The data object
mapped_type obj_;
//- Addressing (next in collision list)
pair_entry* next_;
//- Construct from key, object, next pointer
pair_entry(const Key& key, const T& obj, pair_entry* next)
:
key_(key),
obj_(obj),
next_(next)
{}
//- The key
const key_type& key() const
{
return key_;
}
//- The mapped object
const mapped_type& mapped() const
{
return obj_;
}
mapped_type& mapped()
{
return obj_;
}
private:
//- No copy construct
pair_entry(const pair_entry&) = delete;
//- No copy assignment
void operator=(const pair_entry&) = delete;
};
//- Structure with a single (K) value and a linked-list for collisions
struct unary_entry
{
//- Type of key
typedef Key key_type;
//- Object content type
typedef zero::null mapped_type;
//- Content storage type to the entry
typedef key_type value_type;
//- The lookup key == content
key_type key_;
//- Addressing (next in collision list)
unary_entry* next_;
//- Construct from key, (ununsed) object, next pointer
unary_entry(const Key& key, const T&, unary_entry* next)
:
key_(key),
next_(next)
{}
//- The key
const key_type& key() const
{
return key_;
}
//- Dummy mapped object
const mapped_type& mapped() const
{
return zeroNullElement;
}
mapped_type& mapped()
{
return zeroNullElement;
}
private:
//- No copy construct
unary_entry(const unary_entry&) = delete;
//- No copy assignment
void operator=(const unary_entry&) = delete;
};
//- Hashed node with a linked-list for collisions
//- Table entry. A hashed node with a linked-list for collisions
typedef typename std::conditional
<
std::is_same<zero::null, typename std::remove_cv<T>::type>::value,
unary_entry,
pair_entry
Detail::HashTableSingle<T, Key>,
Detail::HashTablePair<T, Key>
>::type node_type;
......@@ -251,6 +147,10 @@ class HashTable
// No checks for zero-sized tables.
inline label hashKeyIndex(const Key& key) const;
//- Read entry (key, val) and assign
// \return True if the new entry was set.
bool addEntry(Istream& is, const bool overwrite = false);
//- 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);
......@@ -335,7 +235,7 @@ public:
HashTable(this_type&& rhs);
//- Construct from an initializer list
HashTable(std::initializer_list<std::pair<Key, T>> lst);
HashTable(std::initializer_list<std::pair<Key, T>> list);
//- Destructor
......@@ -689,6 +589,9 @@ protected:
//- The key associated with the iterator
inline const Key& key() const;
//- Write the (key, val) pair
inline Ostream& print(Ostream& os) const;
// Member Operators
......@@ -742,7 +645,13 @@ protected:
//- Increment to the next position
inline void increment();
//- The object associated with the iterator
//- The value associated with the iterator
inline mapped_type& val() const
{
return entry_->mapped();
}
//- The object (value) associated with the iterator
inline mapped_type& object() const
{
return entry_->mapped();
......@@ -793,12 +702,15 @@ public:
// Member functions/operators
//- Non-const access to referenced object (value)
using Iterator<false>::val;
//- Non-const access to referenced object
using Iterator<false>::object;
//- Non-const access to referenced object
inline reference operator*() const { return this->object(); }
inline reference operator()() const { return this->object(); }
inline reference operator*() const { return this->val(); }
inline reference operator()() const { return this->val(); }
inline iterator& operator++();
inline iterator operator++(int);
......@@ -852,12 +764,15 @@ public:
// Member functions/operators
//- Const access to referenced object
//- Const access to referenced value
using Iterator<true>::val;
//- Const access to referenced object (value)
using Iterator<true>::object;
//- Const access to referenced object
inline reference operator*() const { return this->object(); }
inline reference operator()() const { return this->object(); }
//- Const access to referenced value
inline reference operator*() const { return this->val(); }
inline reference operator()() const { return this->val(); }
inline const_iterator& operator++();
inline const_iterator operator++(int);
......@@ -966,7 +881,7 @@ public:
friend Istream& operator>> <T, Key, Hash>
(
Istream& is,
HashTable<T, Key, Hash>& L
HashTable<T, Key, Hash>& tbl
);
friend Ostream& operator<< <T, Key, Hash>
......
......@@ -3,7 +3,7 @@
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2011-2012 OpenFOAM Foundation
\\/ M anipulation | Copyright (C) 2017 OpenCFD Ltd.
\\/ M anipulation | Copyright (C) 2017-2019 OpenCFD Ltd.
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
......@@ -30,14 +30,12 @@ License
namespace Foam
{