From 3d4bbcc70368fb5cb9d5715ba948655df8ce96cc Mon Sep 17 00:00:00 2001 From: Quentin Sabah Date: Sat, 16 Oct 2021 14:02:57 +0200 Subject: [PATCH] fix flyweight iterator and enhance test --- .../datastructure/ConcurrentFlyweight.h | 8 +- src/tests/flyweight_test.cpp | 81 +++++++++++-------- 2 files changed, 51 insertions(+), 38 deletions(-) diff --git a/src/include/souffle/datastructure/ConcurrentFlyweight.h b/src/include/souffle/datastructure/ConcurrentFlyweight.h index 129330c9eed..691c1a826a8 100644 --- a/src/include/souffle/datastructure/ConcurrentFlyweight.h +++ b/src/include/souffle/datastructure/ConcurrentFlyweight.h @@ -170,7 +170,8 @@ class ConcurrentFlyweight { NextMaybeUnassignedSlot = END; for (lane_id I = 0; I < This->Lanes.lanes(); ++I) { const auto Lane = This->Lanes.guard(I); - if (This->Handles[I].NextSlot > Slot && This->Handles[I].NextSlot < NextMaybeUnassignedSlot) { + if ((Slot == NONE || This->Handles[I].NextSlot > Slot) && + This->Handles[I].NextSlot < NextMaybeUnassignedSlot) { NextMaybeUnassignedSlot = This->Handles[I].NextSlot; NextMaybeUnassignedHandle = I; } @@ -188,6 +189,7 @@ class ConcurrentFlyweight { bool MoveToNextAssignedSlot() { static_assert(NONE == std::numeric_limits::max(), "required for wrap around to 0 for begin-iterator-scan"); + static_assert(NONE + 1 == 0, "required for wrap around to 0 for begin-iterator-scan"); while (Slot != END) { assert(Slot + 1 < SLOT_MAX); if (Slot + 1 < NextMaybeUnassignedSlot) { // next unassigned slot not reached @@ -206,9 +208,7 @@ class ConcurrentFlyweight { This->Lanes.lock(NextMaybeUnassignedHandle); const bool IsAssigned = (Slot + 1 < This->Handles[NextMaybeUnassignedHandle].NextSlot); This->Lanes.unlock(NextMaybeUnassignedHandle); - if (IsAssigned) { - Slot = Slot + 1; - } + Slot = Slot + 1; FindNextMaybeUnassignedSlot(); if (IsAssigned) { return true; diff --git a/src/tests/flyweight_test.cpp b/src/tests/flyweight_test.cpp index 5d9578c180b..6e94b64c5e0 100644 --- a/src/tests/flyweight_test.cpp +++ b/src/tests/flyweight_test.cpp @@ -27,7 +27,7 @@ // How many times to repeat each randomized test #define RANDOM_TESTS 32 -#define RANDOM_TEST_SIZE 32 +#define RANDOM_TEST_SIZE 64 // For some reason, just using `assert` doesn't work, even if cassert is // included. @@ -53,7 +53,7 @@ static char random_char() { "0123456789" "ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz"; - static constexpr size_t max_index = sizeof(charset) - 1; + static constexpr size_t max_index = sizeof(charset) - 2; std::random_device rd; std::mt19937 mt(rd()); std::uniform_int_distribution dist(0, max_index); @@ -111,30 +111,37 @@ static std::vector::index_type, std::string values.emplace_back(key, s); }; #ifdef _OPENMP -#pragma omp parallel for +#pragma omp parallel + { + srand(omp_get_thread_num()); +#pragma omp for #endif - for (std::size_t i = 0; i < max_size; ++i) { - if (random() % 2) { - add_new(); - } else { - if (random() % 2 && values.size() > 0) { - FlyweightImpl::index_type key; - std::string val; - { - const std::lock_guard lock(mu); - std::tie(key, val) = values[random() % values.size()]; + for (std::size_t i = 0; i < max_size; ++i) { + if (rand() % 2) { + add_new(); + } else { + if (rand() % 2 && values.size() > 0) { + FlyweightImpl::index_type key; + std::string val; + { + const std::lock_guard lock(mu); + std::tie(key, val) = values[rand() % values.size()]; + } + LOCAL_ASSERT_EQ(val, flyweight.fetch(key)); + } else if (values.size() > 0) { + std::string val; + { + const std::lock_guard lock(mu); + val = values[rand() % values.size()].second; + } + LOCAL_ASSERT_TRUE(flyweight.weakContains(val)); } - LOCAL_ASSERT_EQ(val, flyweight.fetch(key)); - } else if (values.size() > 0) { - std::string val; - { - const std::lock_guard lock(mu); - val = values[random() % values.size()].second; - } - LOCAL_ASSERT_TRUE(flyweight.weakContains(val)); } } +#ifdef _OPENMP } +#endif + while (values.size() < min_size) { add_new(); } @@ -144,23 +151,29 @@ static std::vector::index_type, std::string TEST(Flyweight, FindOrInsertCopy) { for (int i = 0; i < RANDOM_TESTS; ++i) { - FlyweightImpl flyweight{1}; + FlyweightImpl flyweight{8}; random_flyweight(flyweight, 0, RANDOM_TEST_SIZE); #ifdef _OPENMP -#pragma omp parallel for +#pragma omp parallel + { + srand(omp_get_thread_num()); +#pragma omp for #endif - for (int j = 0; j < RANDOM_TEST_SIZE; ++j) { - // Guarantee uniqueness by appending something not in the character set - // and then the index. - std::string s = random_string() + "~" + std::to_string(j); - FlyweightImpl::index_type data1, data2; - bool was_new1, was_new2; - std::tie(data1, was_new1) = flyweight.findOrInsert(s); - std::tie(data2, was_new2) = flyweight.findOrInsert(std::string(s)); - EXPECT_EQ(data1, data2); - EXPECT_TRUE(was_new1); - EXPECT_FALSE(was_new2); + for (int j = 0; j < RANDOM_TEST_SIZE; ++j) { + // Guarantee uniqueness by appending something not in the character set + // and then the index. + std::string s = random_string() + "~" + std::to_string(j); + FlyweightImpl::index_type data1, data2; + bool was_new1, was_new2; + std::tie(data1, was_new1) = flyweight.findOrInsert(s); + std::tie(data2, was_new2) = flyweight.findOrInsert(std::string(s)); + EXPECT_EQ(data1, data2); + EXPECT_TRUE(was_new1); + EXPECT_FALSE(was_new2); + } +#ifdef _OPENMP } +#endif } }