From 15f7b1470436c9074b95b0ff4ba63e24a473b27e Mon Sep 17 00:00:00 2001
From: Kruno Tomola Fabro <krunotf@memgraph.io>
Date: Wed, 10 Aug 2016 23:05:24 +0100
Subject: [PATCH] Finished remove method in RhMultiMap. Remove passes tests.
Needs refactoring, has lot of duplicate code.
---
poc/astar.cpp | 31 ++
src/data_structures/map/rh_hashmultimap.hpp | 412 +++++++++-----------
tests/unit/rh_hashmultimap.cpp | 114 +++---
3 files changed, 291 insertions(+), 266 deletions(-)
diff --git a/poc/astar.cpp b/poc/astar.cpp
index 6a4258e1d..f30f1664d 100644
--- a/poc/astar.cpp
+++ b/poc/astar.cpp
@@ -191,6 +191,19 @@ void a_star(Db &db, int64_t sys_id_start, uint max_depth, EdgeFilter e_filter[],
t.commit();
}
+// class Data
+// {
+//
+// private:
+// size_t data = 0;
+// int key;
+//
+// public:
+// Data(int key) : key(key) {}
+//
+// const int &get_key() { return key; }
+// };
+
int main()
{
Db db;
@@ -209,6 +222,24 @@ int main()
double elapsed_ms = (double(end - begin) / CLOCKS_PER_SEC) * 1000;
std::cout << "A-star: " << elapsed_ms << " [ms]\n";
+ // RhHashMultiMap benchmark
+ // const int n_pow2 = 20;
+ // int n = 1 << n_pow2;
+ // RhHashMultiMap<int, Data, n_pow2 + 1> map;
+ // std::srand(time(0));
+ // auto data = std::vector<Data *>();
+ // for (int i = 0; i < n; i++) {
+ // data.push_back(new Data(std::rand()));
+ // }
+ //
+ // begin = clock();
+ // for (auto e : data) {
+ // map.add(e);
+ // }
+ // end = clock();
+ // elapsed_ms = (double(end - begin) / CLOCKS_PER_SEC) * 1000;
+ // std::cout << "Map: " << elapsed_ms << " [ms]\n";
+
return 0;
}
diff --git a/src/data_structures/map/rh_hashmultimap.hpp b/src/data_structures/map/rh_hashmultimap.hpp
index 111c21a3b..671f7d8a5 100644
--- a/src/data_structures/map/rh_hashmultimap.hpp
+++ b/src/data_structures/map/rh_hashmultimap.hpp
@@ -30,7 +30,14 @@ private:
size_t off() { return data & 0x7; }
- void decrement_off() { data--; }
+ bool decrement_off()
+ {
+ if (off() > 0) {
+ data--;
+ return true;
+ }
+ return false;
+ }
bool increment_off()
{
@@ -255,7 +262,7 @@ public:
break;
} else { // Else other has equal or greater
- // offset, so he is poor.
+ // offset, so he is poor.
auto other_key = other.ptr()->get_key();
do {
now = (now + 1) & mask;
@@ -275,6 +282,9 @@ public:
return end();
}
+ // Inserts element.
+ void add(D *data) { add(data->get_key(), data); }
+
// Inserts element with the given key.
void add(const K &key_in, D *data)
{
@@ -287,12 +297,12 @@ public:
size_t start = now;
size_t off = 0;
size_t border = 8 <= capacity ? 8 : capacity;
- bool multi = false;
Combined other = array[now];
while (off < border) {
if (other.valid()) {
auto other_off = other.off();
+ bool multi = false;
if (other_off == off &&
other.ptr()->get_key() == key) { // Found the
do { // same
@@ -321,8 +331,10 @@ public:
}
array[now] = Combined(data, off);
- auto start = now;
- while (adjust_off(other, mask, start, now, multi)) {
+ auto start_insert = now;
+ while (is_off_adjusted(other, mask, start_insert, now,
+ multi) ||
+ other.increment_off()) {
now = (now + 1) & mask;
auto tmp = array[now];
array[now] = other;
@@ -352,244 +364,204 @@ private:
count++;
}
- bool adjust_off(Combined &com, size_t mask, size_t start, size_t now,
- bool multi)
+ // True if no adjusment is needed, false otherwise.
+ bool is_off_adjusted(Combined &com, size_t mask, size_t start, size_t now,
+ bool multi)
{
- if (com.off() == 0) {
- com.increment_off();
- return true;
+ if (com.off() == 0) { // Must be adjusted
+ return false;
}
size_t cin = index(com.ptr()->get_key(), mask);
if ((start <= now && (cin < start || cin > now)) ||
- (now < start && cin < start && cin > now)) {
- return multi || com.increment_off();
+ (now < start && cin < start &&
+ cin > now)) { // Outside [start,now] interval
+ return multi;
}
auto a = array[cin];
auto b = array[(cin + 1) & mask];
- return (a.off() == b.off() &&
- a.ptr()->get_key() == b.ptr()->get_key()) ||
- com.increment_off();
+ return (a.off() == b.off() && a.ptr()->get_key() == b.ptr()->get_key());
+ // Check if different key has eneterd in to
+ // range of other.
}
bool other_poor(Combined other, size_t mask, size_t start, size_t now)
{
auto cin = index(other.ptr()->get_key(), mask);
return (start <= now && (cin <= start || cin > now)) ||
- (now < start && cin <= start && cin > now);
+ (now < start && cin <= start &&
+ cin > now); // If other index is smaller then he is poorer.
+ }
+
+ // True if no adjusment is needed, false otherwise.
+ bool is_off_adjusted_rem(Combined &com, size_t mask, size_t start,
+ size_t bef, size_t now, bool multi)
+ {
+ if (com.off() == 0) { // Must be adjusted
+ return false;
+ }
+ size_t cin = index(com.ptr()->get_key(), mask);
+ if (cin == bef) {
+ return false;
+ }
+ if ((start <= now && (cin < start || cin > now)) ||
+ (now < start && cin < start &&
+ cin > now)) { // Outside [start,now] interval
+ return multi;
+ }
+ auto a = array[cin];
+ auto b = array[before_index(cin, mask)];
+ return b.valid() &&
+ (a.off() == b.off() && a.ptr()->get_key() == b.ptr()->get_key());
+ // Check if different key has eneterd in to
+ // range of other.
}
- // void skip(size_t &now, size_t mask)
- // {
- // Combined start = array[now];
- // size_t end = now;
- // auto off = start.off();
- // auto key = start.ptr()->get_key();
- // do {
- // now = (now + 1) & mask;
- // start = array[now];
- // } while (start.valid() && start.off() == off &&
- // start.ptr()->get_key() == key && now != end);
- // }
- //
- // void _insert(size_t now, Combined com)
- // {
- // Combined other = array[now];
- // array[now] = com;
- // if (other.valid()) {
- // _add(now, other.off(), other.ptr());
- // } else {
- // count++;
- // }
- // }
- //
- // void _add(size_t now, size_t off, Data *data) {
- // size_t mask = this->mask();
- // auto key = std::ref(data->get_key());
- // size_t border = 8 <= capacity ? 8 : capacity;
- //
- // skip()
- //
- // while(off<border){
- // Combined other = array[now];
- // if (other.valid()) {
- // _add(now, other.off(), other.ptr());
- // } else {
- // _insert(now, RhHashMultiMap::Combined com)
- // }
- // }
- //
- // increase_size();
- // add(data);
- // }
public:
- // // Inserts element with the given key.
- // void add(const K &key_in, D *data)
- // {
- // assert(key_in == data->get_key());
- //
- // if (count < capacity) {
- // auto key = std::ref(key_in);
- // size_t mask = this->mask();
- // size_t now = index(key, mask);
- // size_t off = 0;
- //
- // bool bef_init = false;
- // size_t before_off;
- // auto before_key = std::ref(key);
- // bool found_it = false;
- //
- // size_t border = 8 <= capacity ? 8 : capacity;
- // while (off < border) {
- // Combined other = array[now];
- // if (other.valid()) {
- // auto other_off = other.off();
- // auto other_key = std::ref<const
- // K>(other.ptr()->get_key());
- // if (other_off == off && key == other_key) {
- // found_it = true;
- // // Proceed
- //
- // } else if (other_off < off || found_it) { // Other is
- // rich
- // // or after
- // list
- // // of my keys
- // assert(other_off <= off);
- //
- // array[now] = Combined(data, off);
- // // add(other.ptr()->get_key(), other.ptr());
- // // return;
- //
- // // Hacked reusing of function
- // before_off = off;
- // before_key = key;
- // data = other.ptr();
- // key = other_key;
- // off = other_off;
- //
- // if (found_it) { // Offset isn't increased
- // found_it = false;
- // } else {
- // off++;
- // }
- // } else if (bef_init) { // Else other has equal or greater
- // // offset, so he is poor.
- // if (before_off == other_off &&
- // before_key == other_key) {
- // if (count == capacity) {
- // break;
- // }
- // // Proceed
- // } else {
- // before_off = other_off;
- // before_key = other_key;
- // off++;
- // }
- // } else {
- // bef_init = true;
- // before_off = other_off;
- // before_key = other_key;
- // off++;
- // }
- //
- // } else {
- // array[now] = Combined(data, off);
- // count++;
- // return;
- // }
- //
- // now = (now + 1) & mask;
- // }
- // }
- //
- // increase_size();
- // add(data);
- // }
-
- // Inserts element.
- void add(D *data) { add(data->get_key(), data); }
-
// Removes element. Returns removed element if it existed. It doesn't
// specify which element from same key group will be removed.
OptionPtr<D> remove(const K &key_in)
{
- // auto key = std::ref(key_in);
- // size_t mask = this->mask();
- // size_t now = index(key, mask);
- // size_t off = 0;
- //
- // bool bef_init = false;
- // size_t before_off;
- // auto before_key = key;
- // bool found_it = false;
- //
- // size_t border = 8 <= capacity ? 8 : capacity;
- // while (off < border) {
- // Combined other = array[now];
- // if (other.valid()) {
- // auto other_off = other.off();
- // auto other_ptr = other.ptr();
- // auto other_key = std::ref<const K>(other_ptr->get_key());
- // if (other_off == off && key == other_key) { // Found it
- // found_it = true;
- //
- // } else if (found_it) { // Found first element after last
- // element
- // // for remove.
- // auto before = before_index(now, mask);
- // auto ret = OptionPtr<D>(array[before].ptr());
- // std::cout << "<-" << ret.get()->get_key() << "\n";
- // while (other.valid() && other.off() > 0) {
- // std::cout << "<>" << other.ptr()->get_key() << "\n";
- // other.decrement_off();
- // array[before] = other;
- // before = now;
- // now = (now + 1) & mask;
- // other = array[now];
- // }
- //
- // array[before] = Combined();
- // count--;
- // return ret;
- // } else if (other_off < off) { // Other is rich
- // break;
- //
- // } else if (bef_init) { // Else other has equal or greater
- // // offset, so he is poor.
- // if (before_off == other_off && before_key == other_key) {
- // if (count == capacity) { // I am stuck.
- // break;
- // }
- // // Proceed
- // } else {
- // before_off = other_off;
- // before_key = other_key;
- // off++;
- // }
- // } else {
- // bef_init = true;
- // before_off = other_off;
- // before_key = other_key;
- // off++;
- // }
- //
- // } else if (found_it) { // Found empty space after last element
- // for
- // // remove.
- // auto before = before_index(now, mask);
- // auto ret = OptionPtr<D>(array[before].ptr());
- // array[before] = Combined();
- //
- // return ret;
- // } else {
- // break;
- // }
- //
- // now = (now + 1) & mask;
- // }
+
+ if (count > 0) {
+ auto key = std::ref(key_in);
+ size_t mask = this->mask();
+ size_t now = index(key, mask);
+ size_t off = 0;
+ size_t checked = 0;
+ size_t border = 8 <= capacity ? 8 : capacity;
+ Combined other = array[now];
+ while (other.valid() && off < border) {
+ auto other_off = other.off();
+ bool multi = false;
+ if (other_off == off && key == other.ptr()->get_key()) {
+ do {
+ now = (now + 1) & mask;
+ other = array[now];
+ if (!other.valid()) {
+ break;
+ }
+ other_off = other.off();
+ } while (other_off == off &&
+ other.ptr()->get_key() == key &&
+ (multi = true)); // multi = true is correct
+
+ auto bef = before_index(now, mask);
+ auto ret = OptionPtr<D>(array[bef].ptr());
+
+ auto start_rem = bef;
+ while (other.valid() &&
+ (is_off_adjusted_rem(other, mask, start_rem, bef,
+ now, multi) ||
+ other.decrement_off())) {
+ array[bef] = other;
+ bef = now;
+ now = (now + 1) & mask;
+ other = array[now];
+ }
+
+ array[bef] = Combined();
+ count--;
+ return ret;
+
+ } else if (other_off < off) { // Other is rich
+ break;
+
+ } else { // Else other has equal or greater
+ // offset, so he is poor.
+ auto other_key = other.ptr()->get_key();
+ do {
+ now = (now + 1) & mask;
+ other = array[now];
+ checked++;
+ if (checked >= count) { // Reason is possibility of map
+ // full of same values.
+ break;
+ }
+ } while (other.valid() && other.off() == other_off &&
+ other.ptr()->get_key() == other_key);
+ off++;
+ }
+ }
+ }
+
return OptionPtr<D>();
}
+ // Removes element equal by key and value. Returns true if it existed.
+ bool remove(D *data)
+ {
+ if (count > 0) {
+ auto key = std::ref(data->get_key());
+ size_t mask = this->mask();
+ size_t now = index(key, mask);
+ size_t off = 0;
+ size_t checked = 0;
+ size_t border = 8 <= capacity ? 8 : capacity;
+ Combined other = array[now];
+ while (other.valid() && off < border) {
+ auto other_off = other.off();
+ bool multi = false;
+ if (other_off == off && key == other.ptr()->get_key()) {
+ auto founded = capacity;
+ do {
+ if (other.ptr() == data) {
+ founded = now;
+ }
+ now = (now + 1) & mask;
+ other = array[now];
+ if (!other.valid()) {
+ break;
+ }
+ other_off = other.off();
+ } while (other_off == off &&
+ other.ptr()->get_key() == key &&
+ (multi = true)); // multi = true is correct
+ if (founded == capacity) {
+ return false;
+ }
+
+ auto bef = before_index(now, mask);
+ array[founded] = array[bef];
+ // Same as remove of key only diffrence is with founded
+
+ auto start_rem = bef;
+ while (other.valid() &&
+ (is_off_adjusted_rem(other, mask, start_rem, bef,
+ now, multi) ||
+ other.decrement_off())) {
+ array[bef] = other;
+ bef = now;
+ now = (now + 1) & mask;
+ other = array[now];
+ }
+
+ array[bef] = Combined();
+ count--;
+ return true;
+
+ } else if (other_off < off) { // Other is rich
+ break;
+
+ } else { // Else other has equal or greater
+ // offset, so he is poor.
+ auto other_key = other.ptr()->get_key();
+ do {
+ now = (now + 1) & mask;
+ other = array[now];
+ checked++;
+ if (checked >= count) { // Reason is possibility of map
+ // full of same values.
+ break;
+ }
+ } while (other.valid() && other.off() == other_off &&
+ other.ptr()->get_key() == other_key);
+ off++;
+ }
+ }
+ }
+ return false;
+ }
+
void clear()
{
free(array);
diff --git a/tests/unit/rh_hashmultimap.cpp b/tests/unit/rh_hashmultimap.cpp
index 3c19deb7e..39aab9a9b 100644
--- a/tests/unit/rh_hashmultimap.cpp
+++ b/tests/unit/rh_hashmultimap.cpp
@@ -42,18 +42,18 @@ TEST_CASE("Robin hood hashmultimap insert/get check")
REQUIRE(*map.find(0) == ptr0);
}
-// TEST_CASE("Robin hood hasmultihmap remove functionality")
-// {
-// RhHashMultiMap<int, Data> map;
-//
-// REQUIRE(map.find(0) == map.end());
-// auto ptr0 = new Data(0);
-// map.add(ptr0);
-// REQUIRE(map.find(0) != map.end());
-// REQUIRE(*map.find(0) == ptr0);
-// REQUIRE(map.remove(0).get() == ptr0);
-// REQUIRE(map.find(0) == map.end());
-// }
+TEST_CASE("Robin hood hasmultihmap remove functionality")
+{
+ RhHashMultiMap<int, Data> map;
+
+ REQUIRE(map.find(0) == map.end());
+ auto ptr0 = new Data(0);
+ map.add(ptr0);
+ REQUIRE(map.find(0) != map.end());
+ REQUIRE(*map.find(0) == ptr0);
+ REQUIRE(map.remove(0).get() == ptr0);
+ REQUIRE(map.find(0) == map.end());
+}
TEST_CASE("Robin hood hashmultimap double insert")
{
@@ -144,39 +144,62 @@ TEST_CASE("Robin hood hashmultimap checked rand")
cross_validate(map, s_map);
}
-// TEST_CASE("Robin hood hashmultimap with remove checked")
-// {
-// RhHashMultiMap<int, Data> map;
-// std::multimap<int, Data *> s_map;
-//
-// for (int i = 0; i < 2638; i++) {
-// int key = (std::rand() % 100) << 3;
-// if ((std::rand() % 3) == 0) {
-// std::cout << "Remove: " << key << "\n";
-// auto removed = map.remove(key);
-// // auto it = s_map.find(key);
-// if (removed.is_present()) {
-// // while (it != s_map.end() && it->second != removed.get()) {
-// // it++;
-// // }
-// // REQUIRE(it != s_map.end());
-// // s_map.erase(it);
-// // cross_validate(map, s_map);
-//
-// } else {
-// // REQUIRE(it == s_map.end());
-// }
-// } else {
-// std::cout << "Insert: " << key << "\n";
-// auto data = new Data(key);
-// map.add(data);
-// s_map.insert(std::pair<int, Data *>(key, data));
-// cross_validate(map, s_map);
-// }
-// }
-//
-// cross_validate_weak(map, s_map);
-// }
+TEST_CASE("Robin hood hashmultimap with remove checked")
+{
+ RhHashMultiMap<int, Data> map;
+ std::multimap<int, Data *> s_map;
+
+ std::srand(std::time(0));
+ for (int i = 0; i < 162638; i++) {
+ int key = (std::rand() % 10000) << 3;
+ if ((std::rand() % 3) == 0) {
+ auto removed = map.remove(key);
+ auto it = s_map.find(key);
+ if (removed.is_present()) {
+ while (it != s_map.end() && it->second != removed.get()) {
+ it++;
+ }
+ REQUIRE(it != s_map.end());
+ s_map.erase(it);
+
+ } else {
+ REQUIRE(it == s_map.end());
+ }
+ } else {
+ auto data = new Data(key);
+ map.add(data);
+ s_map.insert(std::pair<int, Data *>(key, data));
+ }
+ }
+
+ cross_validate(map, s_map);
+}
+
+TEST_CASE("Robin hood hashmultimap with remove data checked")
+{
+ RhHashMultiMap<int, Data> map;
+ std::multimap<int, Data *> s_map;
+
+ std::srand(std::time(0));
+ for (int i = 0; i < 162638; i++) {
+ int key = (std::rand() % 10000) << 3;
+ if ((std::rand() % 2) == 0) {
+ auto it = s_map.find(key);
+ if (it == s_map.end()) {
+ REQUIRE(map.find(key) == map.end());
+ } else {
+ s_map.erase(it);
+ REQUIRE(map.remove(it->second));
+ }
+ } else {
+ auto data = new Data(key);
+ map.add(data);
+ s_map.insert(std::pair<int, Data *>(key, data));
+ }
+ }
+
+ cross_validate(map, s_map);
+}
void cross_validate(RhHashMultiMap<int, Data> &map,
std::multimap<int, Data *> &s_map)
@@ -193,7 +216,6 @@ void cross_validate(RhHashMultiMap<int, Data> &map,
for (auto e : s_map) {
auto it = map.find(e.first);
- // std::cout << "s_map: " << e.first << "\n";
while (it != map.end() && *it != e.second) {
it++;