diff --git a/ApiTest.cpp b/ApiTest.cpp new file mode 100644 index 0000000..c04b8e0 --- /dev/null +++ b/ApiTest.cpp @@ -0,0 +1,90 @@ +#include "VersionedMap.h" + +#include "Internal.h" +#include "PrintMutation.h" + +#include +#include +#include +#include +#include + +void setAndClearPrev() { + debugVerboseEnabled = false; + weaselab::VersionedMap versionedMap{0}; + + auto setAndClear = [&versionedMap](int64_t version) { + const int64_t k = __builtin_bswap64(version); + const int64_t zero = __builtin_bswap64(0); + weaselab::VersionedMap::Mutation m[] = { + {(const uint8_t *)&zero, (const uint8_t *)&k, 8, 8, + weaselab::VersionedMap::Clear}, + {(const uint8_t *)&k, (const uint8_t *)&k, 8, 8, + weaselab::VersionedMap::Set}, + }; + versionedMap.addMutations(m, sizeof(m) / sizeof(m[0]), version); + assert(versionedMap.getVersion() == version); + }; + + int64_t version = 0; + while (version < 1000) { + ++version; + setAndClear(version); + } + + std::latch ready{2}; + + auto writer = std::thread{[&]() { + ready.arrive_and_wait(); + + int64_t version = 1000; + while (version < 100000) { + ++version; + setAndClear(version); + } + }}; + + ready.arrive_and_wait(); + + version = 0; + while (version < 1000) { + ++version; + + auto iter = versionedMap.begin(version); + + auto m = *iter++; + int64_t k = __builtin_bswap64(version); + assert(m.version == version); + assert(m.param1Len == 8); + assert(m.param2Len == 8); + int64_t zero = __builtin_bswap64(0); + assert(memcmp(m.param1, &zero, 8) == 0); + assert(memcmp(m.param2, &k, 8) == 0); + assert(m.type == weaselab::VersionedMap::Clear); + + m = *iter++; + assert(m.version == version); + assert(m.param1Len == 8); + assert(m.param2Len == 8); + assert(memcmp(m.param1, &k, 8) == 0); + assert(memcmp(m.param2, &k, 8) == 0); + assert(m.type == weaselab::VersionedMap::Set); + + assert(iter == versionedMap.end(version)); + } + + writer.join(); + + debugVerboseEnabled = true; + printf("Before gc: %" PRId64 "\n", versionedMap.getBytes()); + versionedMap.setOldestVersion(versionedMap.getVersion()); + printf("After gc: %" PRId64 "\n", versionedMap.getBytes()); + assert(versionedMap.getOldestVersion() == versionedMap.getVersion()); + for (auto iter = versionedMap.begin(versionedMap.getVersion()), + end = versionedMap.end(versionedMap.getVersion()); + iter != end; ++iter) { + printMutation(*iter); + } +} + +int main() { setAndClearPrev(); } \ No newline at end of file diff --git a/CMakeLists.txt b/CMakeLists.txt index 4716306..c409ba1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -87,12 +87,12 @@ add_custom_command( COMMAND_EXPAND_LISTS) add_library(${PROJECT_NAME} SHARED ${CMAKE_BINARY_DIR}/${PROJECT_NAME}.o) +target_include_directories( + ${PROJECT_NAME} PUBLIC $) set_target_properties( ${PROJECT_NAME} PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/versioned-map") -if(NOT CMAKE_BUILD_TYPE STREQUAL Debug) - set_target_properties(${PROJECT_NAME} PROPERTIES LINKER_LANGUAGE C) -endif() +set_target_properties(${PROJECT_NAME} PROPERTIES LINKER_LANGUAGE C) if(HAS_VERSION_SCRIPT) target_link_options(${PROJECT_NAME} PRIVATE @@ -100,9 +100,9 @@ if(HAS_VERSION_SCRIPT) endif() add_library(${PROJECT_NAME}-static STATIC ${CMAKE_BINARY_DIR}/${PROJECT_NAME}.o) -if(NOT CMAKE_BUILD_TYPE STREQUAL Debug) - set_target_properties(${PROJECT_NAME}-static PROPERTIES LINKER_LANGUAGE C) -endif() +target_include_directories( + ${PROJECT_NAME}-static PUBLIC $) +set_target_properties(${PROJECT_NAME}-static PROPERTIES LINKER_LANGUAGE C) if(NOT APPLE) add_custom_command( @@ -115,6 +115,8 @@ if(NOT APPLE) "Proceeding with all symbols global in static library") endif() +set(TEST_FLAGS -Wall -Wextra -Wunreachable-code -Wpedantic -UNDEBUG) + include(CTest) option(BUILD_TREE_VIS "Build tree visualization" OFF) @@ -130,16 +132,24 @@ if(BUILD_TESTING) add_executable(rootset_test RootSet.cpp) target_compile_definitions(rootset_test PRIVATE ENABLE_ROOTSET_TESTS) target_compile_options(rootset_test PRIVATE -fsanitize=address,undefined - -UNDEBUG) + ${TEST_FLAGS}) target_link_options(rootset_test PRIVATE -fsanitize=address,undefined) add_test(NAME rootset_test COMMAND rootset_test) add_executable(rootset_test_tsan RootSet.cpp) target_compile_definitions(rootset_test_tsan PRIVATE ENABLE_ROOTSET_TESTS) - target_compile_options(rootset_test_tsan PRIVATE -fsanitize=thread -UNDEBUG) + target_compile_options(rootset_test_tsan PRIVATE -fsanitize=thread + ${TEST_FLAGS}) target_link_options(rootset_test_tsan PRIVATE -fsanitize=thread) add_test(NAME rootset_test_tsan COMMAND rootset_test) + add_executable(api_test ApiTest.cpp) + target_link_libraries(api_test PRIVATE ${PROJECT_NAME}) + target_compile_options(api_test PRIVATE -fsanitize=address,undefined + ${TEST_FLAGS}) + target_link_options(api_test PRIVATE -fsanitize=address,undefined) + add_test(NAME api_test COMMAND api_test) + # symbol visibility tests if(NOT CMAKE_BUILD_TYPE STREQUAL Debug) if(APPLE) diff --git a/Internal.h b/Internal.h index 8f093c2..5fcc140 100644 --- a/Internal.h +++ b/Internal.h @@ -17,10 +17,8 @@ #define DEBUG_VERBOSE 0 #endif -#if DEBUG_VERBOSE // Use to toggle debug verbose dynamically inline bool debugVerboseEnabled = true; -#endif // This header contains code that we want to reuse outside of ConflictSet.cpp or // want to exclude from coverage since it's only testing related. diff --git a/PrintMutation.h b/PrintMutation.h new file mode 100644 index 0000000..2803d73 --- /dev/null +++ b/PrintMutation.h @@ -0,0 +1,35 @@ +#pragma once + +#include "VersionedMap.h" +#include +#include + +inline void +printMutation(const weaselab::VersionedMap::Iterator::VersionedMutation &m) { + switch (m.type) { + case weaselab::VersionedMap::Set: + printf("set "); + for (int i = 0; i < m.param1Len; ++i) { + printf("x%02x", m.param1[i]); + } + printf(" -> '"); + for (int i = 0; i < m.param2Len; ++i) { + printf("x%02x", m.param2[i]); + } + printf("' @ %" PRId64 "\n", m.version); + break; + case weaselab::VersionedMap::Clear: + printf("clear ["); + for (int i = 0; i < m.param1Len; ++i) { + printf("x%02x", m.param1[i]); + } + printf(", "); + for (int i = 0; i < m.param2Len; ++i) { + printf("x%02x", m.param2[i]); + } + printf(") @ %" PRId64 "\n", m.version); + break; + default: // GCOVR_EXCL_LINE + __builtin_unreachable(); // GCOVR_EXCL_LINE + } +} \ No newline at end of file diff --git a/RootSet.cpp b/RootSet.cpp index 7277292..8012d94 100644 --- a/RootSet.cpp +++ b/RootSet.cpp @@ -188,14 +188,14 @@ int main() { RootSet rs; std::latch ready{1 + kNumReaders}; - std::atomic version; - std::vector> doneVersions(kNumReaders); + std::atomic version; + std::vector> doneVersions(kNumReaders); std::thread writer([&]() { ready.arrive_and_wait(); for (int i = 0; i < kNumVersions; ++i) { rs.add(i / 10, i); version.store(i); - int min = std::numeric_limits::max(); + uint32_t min = -1; for (auto &v : doneVersions) { min = std::min(min, v.load()); } diff --git a/VersionedMap.cpp b/VersionedMap.cpp index 9442b02..3d78656 100644 --- a/VersionedMap.cpp +++ b/VersionedMap.cpp @@ -427,33 +427,32 @@ struct Finger { void setSearchPathSizeUnsafe(int size) { searchPathSize_ = size; } - Finger() : searchPathSize_(0) {} + Finger() { clear(); } - Finger(const Finger &other) { + void clear() { #ifndef NDEBUG memset(searchPath, 0, sizeof(searchPath)); memset(direction, 0, sizeof(direction)); #endif - memcpy(searchPath, other.searchPath, - other.searchPathSize_ * sizeof(searchPath[0])); - memcpy(direction, other.direction, - other.searchPathSize_ * sizeof(direction[0])); - searchPathSize_ = other.searchPathSize_; + searchPathSize_ = 0; } - Finger &operator=(const Finger &other) { + void copyTo(Finger &result) { #ifndef NDEBUG - memset(searchPath, 0, sizeof(searchPath)); - memset(direction, 0, sizeof(direction)); + memset(result.searchPath, 0, sizeof(searchPath)); + memset(result.direction, 0, sizeof(direction)); #endif - memcpy(searchPath, other.searchPath, - other.searchPathSize_ * sizeof(searchPath[0])); - memcpy(direction, other.direction, - other.searchPathSize_ * sizeof(direction[0])); - searchPathSize_ = other.searchPathSize_; - return *this; + memcpy(result.searchPath, searchPath, + searchPathSize_ * sizeof(searchPath[0])); + memcpy(result.direction, direction, searchPathSize_ * sizeof(direction[0])); + result.searchPathSize_ = searchPathSize_; } + Finger(const Finger &) = delete; + Finger &operator=(const Finger &) = delete; + Finger(Finger &&) = delete; + Finger &operator=(Finger &&) = delete; + private: uint32_t searchPath[kPathLengthUpperBound]; bool direction[kPathLengthUpperBound]; @@ -483,6 +482,7 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { template uint32_t child(uint32_t node, bool which, int64_t at) const { + assert(node != 0); static_assert(kOrder == std::memory_order_acquire || kOrder == std::memory_order_relaxed); auto &n = mm.base[node]; @@ -579,11 +579,11 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { }; template - Finger search(Key key, T root, int64_t version) const { + void search(Key key, T root, int64_t version, Finger &finger) const { + finger.clear(); static_assert(std::is_same_v); - Finger finger; if (root == 0) { - return finger; + return; } bool ignored; finger.push(root, ignored); @@ -601,7 +601,6 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { } finger.push(child(n, c > 0, version), c > 0); } - return finger; } // If `val` is set, then this is a point mutation at `latestVersion`. @@ -633,7 +632,8 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { if (val.has_value()) { pointVersion = latestVersion; if (inserted) { - auto copy = finger; + Finger copy; + finger.copyTo(copy); move(copy, latestVersion, true); if (copy.searchPathSize() == 0) { rangeVersion = -1; @@ -694,6 +694,16 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { // entry. void remove(Finger &finger) { +#ifndef NDEBUG + uint32_t expected; + { + Finger copy; + finger.copyTo(copy); + move(copy, latestVersion, true); + expected = copy.searchPathSize() > 0 ? copy.backNode() : 0; + } +#endif + // True if finger is pointing to an entry > than the entry we're removing // after we rotate it down bool greaterThan; @@ -743,7 +753,7 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { } finger.setSearchPathSizeUnsafe(oldSize); - if (greaterThan) { + if (greaterThan && finger.backNode() != 0) { uint32_t c; while ((c = child(finger.backNode(), false, latestVersion)) != 0) { @@ -753,10 +763,11 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { move(finger, latestVersion, true); } - if (finger.backNode() == 0) { - finger.pop(); - assert(finger.searchPathSize() == 0); +#ifndef NDEBUG + if (finger.searchPathSize() > 0) { + assert(finger.backNode() == expected); } +#endif } uint32_t newNode(int64_t version, int64_t rangeVersion, const uint8_t *key, @@ -802,8 +813,9 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { case Clear: { insert({m.param1, m.param1Len}, {{nullptr, -1}}); if (m.param2Len > 0) { - auto iter = search( - {m.param1, m.param1Len}, latestRoot, latestVersion); + Finger iter; + search({m.param1, m.param1Len}, latestRoot, + latestVersion, iter); move(iter, latestVersion, true); while (iter.searchPathSize() > 0 && mm.base[iter.backNode()] < Key{m.param2, m.param2Len}) { @@ -881,6 +893,17 @@ struct VersionedMap::Iterator::Impl { int mutationCount; int mutationIndex; VersionedMutation mutations[2]; + + void copyTo(Impl &result) { + result.cmp = cmp; + result.map = map; + result.version = version; + result.mutationCount = mutationCount; + result.mutationIndex = mutationIndex; + result.mutations[0] = mutations[0]; + result.mutations[1] = mutations[1]; + finger.copyTo(result.finger); + } }; VersionedMap::Iterator::~Iterator() { @@ -891,7 +914,9 @@ VersionedMap::Iterator::~Iterator() { } VersionedMap::Iterator::Iterator(const Iterator &other) - : impl(new(safe_malloc(sizeof(Impl))) Impl(*other.impl)) {} + : impl(new(safe_malloc(sizeof(Impl))) Impl()) { + other.impl->copyTo(*impl); +} VersionedMap::Iterator & VersionedMap::Iterator::operator=(const Iterator &other) { @@ -899,7 +924,8 @@ VersionedMap::Iterator::operator=(const Iterator &other) { impl->~Impl(); safe_free(impl, sizeof(*impl)); } - impl = new (safe_malloc(sizeof(Impl))) Impl(*other.impl); + impl = new (safe_malloc(sizeof(Impl))) Impl(); + other.impl->copyTo(*impl); return *this; } @@ -932,7 +958,8 @@ VersionedMap::Iterator::operator*() const { void materializeMutations(VersionedMap::Iterator::Impl *impl, const Entry *prev, const Entry *next) { if (prev == nullptr) { - auto copy = impl->finger; + Finger copy; + impl->finger.copyTo(copy); impl->map->move(copy, impl->version, false); if (copy.searchPathSize() > 0) { prev = impl->map->mm.base[copy.backNode()].entry; @@ -941,7 +968,8 @@ void materializeMutations(VersionedMap::Iterator::Impl *impl, const Entry *prev, } } if (next == nullptr) { - auto copy = impl->finger; + Finger copy; + impl->finger.copyTo(copy); impl->map->move(copy, impl->version, true); if (copy.searchPathSize() > 0) { next = impl->map->mm.base[copy.backNode()].entry; @@ -1058,7 +1086,8 @@ void VersionedMap::Impl::firstGeq(const Key *key, const int64_t *version, iterator[i].impl = new (safe_malloc(sizeof(Iterator::Impl))) Iterator::Impl(); } - auto finger = search(key[i], root, version[i]); + Finger &finger = iterator[i].impl->finger; + search(key[i], root, version[i], finger); if (finger.searchPathSize() == 0) { iterator[i].impl->cmp = 1; @@ -1072,13 +1101,12 @@ void VersionedMap::Impl::firstGeq(const Key *key, const int64_t *version, iterator[i].impl->cmp = 0; } - iterator[i].impl->finger = finger; iterator[i].impl->version = version[i]; iterator[i].impl->map = this; const Entry *prev = nullptr; for (;;) { - if (iterator[i].impl->finger.searchPathSize() > 0) { + if (finger.searchPathSize() > 0) { materializeMutations(iterator[i].impl, prev, nullptr); if (iterator[i].impl->mutationCount > 0) { break; @@ -1086,11 +1114,9 @@ void VersionedMap::Impl::firstGeq(const Key *key, const int64_t *version, } else { break; } - prev = iterator[i] - .impl->map->mm.base[iterator[i].impl->finger.backNode()] - .entry; + prev = iterator[i].impl->map->mm.base[finger.backNode()].entry; iterator[i].impl->map->move( - iterator[i].impl->finger, iterator[i].impl->version, true); + finger, iterator[i].impl->version, true); } if (iterator[i].impl->cmp == 0) { iterator[i].impl->mutationIndex = iterator[i].impl->mutationCount - 1; @@ -1230,6 +1256,8 @@ struct __attribute__((visibility("default"))) PeakPrinter { #ifdef ENABLE_MAIN #include +#include "PrintMutation.h" + void breakpoint_me() {} int main() { @@ -1272,33 +1300,7 @@ int main() { printf("Bytes: %" PRId64 "\n", versionedMap.getBytes()); breakpoint_me(); for (auto end = versionedMap.end(v); iter != end; ++iter) { - const auto &m = *iter; - switch (m.type) { - case weaselab::VersionedMap::Set: - printf("set "); - for (int i = 0; i < m.param1Len; ++i) { - printf("x%02x", m.param1[i]); - } - printf(" -> '"); - for (int i = 0; i < m.param2Len; ++i) { - printf("x%02x", m.param2[i]); - } - printf("' @ %" PRId64 "\n", m.version); - break; - case weaselab::VersionedMap::Clear: - printf("clear ["); - for (int i = 0; i < m.param1Len; ++i) { - printf("x%02x", m.param1[i]); - } - printf(", "); - for (int i = 0; i < m.param2Len; ++i) { - printf("x%02x", m.param2[i]); - } - printf(") @ %" PRId64 "\n", m.version); - break; - default: // GCOVR_EXCL_LINE - __builtin_unreachable(); // GCOVR_EXCL_LINE - } + printMutation(*iter); } } return 0;