Skip to content

Tuneups #2

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.

Sign up for GitHub

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

Open
apoc wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Tuneups #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8748f5
* Improvements in HashMap:
Jan 26, 2017
6fd44de
* Just some constants
Jan 26, 2017
0bc309e
* Added assert to check hash size
Jan 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 29 additions & 37 deletions inc/HashMap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
#ifndef HASH_MAP_H_
#define HASH_MAP_H_

#include <cstdint>
#include <iostream>
#include <cstdint>
#include <cassert>
#include <iostream>
#include <functional>
#include <mutex>
#include "HashNode.h"
#include <mutex>
#include "HashNode.h"

constexpr size_t HASH_SIZE_DEFAULT = 2048;

constexpr size_t HASH_SIZE_DEFAULT = 1031; // A prime number as hash size gives a better distribution of values in buckets
namespace CTSL //Concurrent Thread Safe Library
{
//The class represting the hash map.
Expand All @@ -18,76 +20,66 @@ namespace CTSL //Concurrent Thread Safe Library
//Each hash bucket is implemented as singly linked list with the head as a dummy node created
//during the creation of the bucket. All the hash buckets are created during the construction of the map.
//Locks are taken per bucket, hence multiple threads can write simultaneously in different buckets in the hash map
template <typename K, typename V, typename F = std::hash<K> >
template <typename K, typename V, typename HASH = std::hash<K> >
class HashMap
{
public:
HashMap(size_t hashSize_ = HASH_SIZE_DEFAULT) : hashSize(hashSize_)
{
hashTable = new HashBucket<K, V> * [hashSize](); //create the hash table as an array of hash buckets
for(size_t i = 0; i < hashSize; i++)
{
hashTable[i] = new HashBucket<K, V>(); //create the hash buckets
}
assert(hashSize_ > 0 && "Hash size muste be > 0");
assert((hashSize_ & (hashSize_ - 1)) == 0 && "Hash size must be power of 2");
Copy link

@dbseraf dbseraf Feb 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seriously, if you're going to change the API contract, you really must update the documentation.

Copy link
Author

@apoc apoc Mar 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What API constract and documentation are you talking about?


hashTable = new HashBucket<K, V>[hashSize]; //create the hash table as an array of hash buckets
}

~HashMap()
{
for(size_t i = 0; i < hashSize; i++)
{
delete hashTable[i]; //delete all the hash buckets
}

delete []hashTable;
delete [] hashTable;
}

//Copy and Move of the HashMap are not supported at this moment
HashMap(const HashMap&) = delete;
HashMap(HashMap&&) = delete;
HashMap& operator=(const HashMap&) = delete;
HashMap& operator=(const HashMap&) = delete;
HashMap& operator=(HashMap&&) = delete;

//Function to find an entry in the hash map matching the key.
//If key is found, the corresponding value is copied into the parameter "value" and function returns true.
//If key is not found, function returns false.
bool find(const K &key, V &value) const
bool find(const K &key, V &value) const
{
size_t hashValue = hashFn(key) % hashSize ;
HashBucket<K, V> * bucket = hashTable[hashValue];
return bucket->find(key, value);
const size_t hashValue = HASH()(key) & (hashSize - 1); // Only for hash size power of 2
return hashTable[hashValue].find(key, value);
}

//Function to insert into the hash map.
//If key already exists, update the value, else insert a new node in the bucket with the <key, value> pair.
void insert(const K &key, const V &value)
{
size_t hashValue = hashFn(key) % hashSize ;
HashBucket<K, V> * bucket = hashTable[hashValue];
bucket->insert(key, value);
const size_t hashValue = HASH()(key) & (hashSize - 1); // Only for hash size power of 2
hashTable[hashValue].insert(key, value);
}

//Function to remove an entry from the bucket, if found
void erase(const K &key)
void erase(const K &key)
{
size_t hashValue = hashFn(key) % hashSize ;
HashBucket<K, V> * bucket = hashTable[hashValue];
bucket->erase(key);
}
const size_t hashValue = HASH()(key) & (hashSize - 1); // Only for hash size power of 2
hashTable[hashValue].erase(key);
}

//Function to clean up the hasp map, i.e., remove all entries from it
void clear()
{
for(size_t i = 0; i < hashSize; i++)
{
(hashTable[i])->clear();
hashTable[i].clear();
}
}
}

private:


HashBucket<K, V> ** hashTable;
F hashFn;
size_t hashSize;
HashBucket<K, V>* hashTable;
const size_t hashSize;
};
}
#endif
Expand Down
102 changes: 51 additions & 51 deletions inc/HashNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@ namespace CTSL //Concurrent Thread Safe Library
class HashNode
{
public:
HashNode() : next(nullptr)
{};
HashNode(K key_, V value_) : next(nullptr), key(key_), value(value_)
{};
~HashNode() {next = nullptr;}
HashNode() : next(nullptr) {
}

HashNode(K key_, V value_) : next(nullptr), key(key_), value(value_) {
}

~HashNode()
{
next = nullptr;
}

const K& getKey() const {return key;}
void setValue(V value_) {value = value_;}
Expand All @@ -32,34 +37,33 @@ namespace CTSL //Concurrent Thread Safe Library
class HashBucket
{
public:
HashBucket()
HashBucket() : head(nullptr)
{
head = new HashNode<K, V>();//A bucket is created with its head as a dummy hash node.
}

~HashBucket() //delete the bucket
{
std::unique_lock<std::shared_timed_mutex> lock(mutex_); //take a lock before removing the nodes
HashNode<K, V> * prev = nullptr;
HashNode<K, V> * node = head;
while(node != nullptr)

while(head)
{
prev = node;
node = node->next;
delete prev;
HashNode<K, V>* next = head->next;
delete head;

head = next;
}
}
}

//Function to find an entry in the bucket matching the key
//If key is found, the corresponding value is copied into the parameter "value" and function returns true.
//If key is not found, function returns false
bool find(const K &key, V &value) const
{
// A shared mutex is used to enable mutiple concurrent reads
std::shared_lock<std::shared_timed_mutex> lock(mutex_);
HashNode<K, V> * node = head->next; //The head node is dummy, no need to look there
std::shared_lock<std::shared_timed_mutex> lock(mutex_);
HashNode<K, V> * node = head;

while (node != nullptr)
while (node)
{
if (node->getKey() == key)
{
Expand All @@ -77,48 +81,44 @@ namespace CTSL //Concurrent Thread Safe Library
{
//Exclusive lock to enable single write in the bucket
std::unique_lock<std::shared_timed_mutex> lock(mutex_);
HashNode<K, V> * prev = head;
HashNode<K, V> * node = head->next; //The head node is dummy, no need to look there
HashNode<K, V>** node = &head;

while (node != nullptr && node->getKey() != key)
while (*node)
{
prev = node;
node = node->next;
if ((*node)->getKey() == key)
{
(*node)->setValue(value); //Key found in bucket, update the value
return;
}
node = &((*node)->next);
}

if (nullptr == node)
{
prev->next = new HashNode<K, V>(key, value); //New entry, create a node and add to bucket
}
else
{
node->setValue(value); //Key found in bucket, update the value
}

// now node points to lasts element ->next or head so we can construct the new object unconditionaly here :)
*node = new HashNode<K, V>(key, value);
}

//Function to remove an entry from the bucket, if found
void erase(const K &key)
{
//Exclusive lock to enable single write in the bucket
std::unique_lock<std::shared_timed_mutex> lock(mutex_);
HashNode<K, V> *prev = head;
HashNode<K, V> * node = head->next;

while (node != nullptr && node->getKey() != key)
{
prev = node;
node = node->next;
}
HashNode<K, V>* node = head;
HashNode<K, V>** prev = &head;

if (nullptr == node) //Key not found, nothing to be done
while (node)
{
return;
}
else
{
prev->next = node->next; //Remove the node from the bucket
delete node; //Free up the memory
if (node->getKey() == key)
{
*prev = node->next;
delete node;
return;
}
else
{
prev = &node;
node = node->next;
}
}

}
Expand All @@ -129,13 +129,13 @@ namespace CTSL //Concurrent Thread Safe Library
{
//Exclusive lock to enable single write in the bucket
std::unique_lock<std::shared_timed_mutex> lock(mutex_);
HashNode<K, V> * prev = head;
HashNode<K, V> * node = head->next; //The dummy head node will not be removed
while(node != nullptr)

while(head)
{
prev->next = node->next;
delete node;
node = prev->next;
HashNode<K, V>* toDel = head;
delete toDel;

head = head->next;
}
}

Expand Down
14 changes: 7 additions & 7 deletions src/HastTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

void testSingleThreadStringKey()
{
CTSL::HashMap<std::string, int> stringMap(1001);
CTSL::HashMap<std::string, int> stringMap(2048);

stringMap.insert("test1", 200);
stringMap.insert("test2", 670);
Expand Down Expand Up @@ -37,7 +37,7 @@ void testSingleThreadStringKey()
{
std::cout << "Did not find value for key \"test1\"" << std::endl;
}

stringMap.erase("test1");
if(stringMap.find("test1", value))
{
Expand Down Expand Up @@ -72,7 +72,7 @@ void testSingleThreadStringKey()

void testSingleThreadIntegerKey()
{
CTSL::HashMap<int, int> integerMap(29);
CTSL::HashMap<int, int> integerMap(32);

integerMap.insert(10, 200);
integerMap.insert(20, 670);
Expand Down Expand Up @@ -105,7 +105,7 @@ void testSingleThreadIntegerKey()
{
std::cout << "Did not find value for key 20" << std::endl;
}

integerMap.erase(20);
if(integerMap.find(20, value))
{
Expand Down Expand Up @@ -179,7 +179,7 @@ void testMultiThreadIntegerKey_Func1(CTSL::HashMap<int, int> &integerMap)
{
std::cout << "Thread 1: Did not find value for key 20 in HashMap " << &integerMap << std::endl;
}

integerMap.erase(20);
std::this_thread::sleep_for(std::chrono::seconds(1));
if(integerMap.find(20, value))
Expand Down Expand Up @@ -249,7 +249,7 @@ void testMultiThreadIntegerKey_Func2(CTSL::HashMap<int, int> &integerMap)
{
std::cout << "Thread 2: Did not find value for key 20 in HashMap " << &integerMap << std::endl;
}

integerMap.erase(20);
if(integerMap.find(20, value))
{
Expand Down Expand Up @@ -284,7 +284,7 @@ void testMultiThreadIntegerKey_Func2(CTSL::HashMap<int, int> &integerMap)
int main()
{
testSingleThread(); //Single threaded test

//Multi threaded test with two threads
CTSL::HashMap<int, int> integerMap;
std::thread firstThread(testMultiThreadIntegerKey_Func1, ref(integerMap));
Expand Down