From 685b49c96dce6ec2fc8bc2d57632a58c27cd8ae8 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Fri, 14 Jun 2024 17:58:33 -0700 Subject: [PATCH] WIP adhoc test looks ok --- PrintMutation.h | 52 +++++++------------- VersionedMap.cpp | 122 ++++++++++++++++++++++++++++++++--------------- 2 files changed, 101 insertions(+), 73 deletions(-) diff --git a/PrintMutation.h b/PrintMutation.h index 9f6164a..eb1769d 100644 --- a/PrintMutation.h +++ b/PrintMutation.h @@ -1,56 +1,40 @@ #pragma once #include "VersionedMap.h" +#include #include #include #include +inline void printBinary(const weaselab::VersionedMap::Key k) { + for (int i = 0; i < k.len; ++i) { + auto c = k.p[i]; + if (isprint(c)) { + printf("%c", c); + } else { + printf("x%02x", c); + } + } +} + 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) { - auto c = m.param1[i]; - if (isprint(c)) { - printf("%c", c); - } else { - printf("x%02x", c); - } - } + printBinary({m.param1, m.param1Len}); printf(" -> '"); - for (int i = 0; i < m.param2Len; ++i) { - auto c = m.param2[i]; - if (isprint(c)) { - printf("%c", c); - } else { - printf("x%02x", c); - } - } + printBinary({m.param2, m.param2Len}); printf("' @ %" PRId64 "\n", m.notModifiedSince); break; case weaselab::VersionedMap::Clear: printf("clear ["); - for (int i = 0; i < m.param1Len; ++i) { - auto c = m.param1[i]; - if (isprint(c)) { - printf("%c", c); - } else { - printf("x%02x", c); - } - } + printBinary({m.param1, m.param1Len}); printf(", "); - for (int i = 0; i < m.param2Len; ++i) { - auto c = m.param2[i]; - if (isprint(c)) { - printf("%c", c); - } else { - printf("x%02x", c); - } - } + printBinary({m.param2, m.param2Len}); printf(") @ %" PRId64 "\n", m.notModifiedSince); break; - default: // GCOVR_EXCL_LINE - __builtin_unreachable(); // GCOVR_EXCL_LINE + default: // GCOVR_EXCL_LINE + assert(false); // GCOVR_EXCL_LINE } } \ No newline at end of file diff --git a/VersionedMap.cpp b/VersionedMap.cpp index 0e8da7f..962c4d5 100644 --- a/VersionedMap.cpp +++ b/VersionedMap.cpp @@ -1,6 +1,7 @@ #include "VersionedMap.h" #include "Internal.h" #include "KeyCompare.h" +#include "PrintMutation.h" #include "RootSet.h" #include @@ -478,6 +479,13 @@ private: int searchPathSize_; }; +VersionedMap::Key keyAfter(VersionedMap::Key k, Arena &arena) { + uint8_t *result = new (arena) uint8_t[k.len + 1]; + memcpy(result, k.p, k.len); + result[k.len] = 0; + return {result, k.len + 1}; +} + struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { // The last node is allowed to be 0, in which case this is the search path of @@ -616,15 +624,17 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { } } - // If `val` is set, then this is a point mutation at `latestVersion`. - // Otherwise it's the end of a range mutation at `latestVersion`. + // If `val` is true, then this is a point set at `latestVersion`. + // If `endRange` is true, then this is a range end marker at `latestVersion`. + // Otherwise it's the beginning of a range at `latestVersion`. // `finger` is a valid finger to the insertion path of `key` in the latest // version (which can be obtained with `search`) - void insert(Key key, std::optional val, Finger &finger) { + void insert(Key key, std::optional val, bool endRange, Finger &finger) { const bool inserted = finger.backNode() == 0; int64_t pointVersion, rangeVersion; if (val.has_value()) { + // Point set pointVersion = latestVersion; if (inserted) { Finger copy; @@ -639,25 +649,47 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { auto *entry = mm.base[finger.backNode()].entry; rangeVersion = entry->rangeVersion; } - } else { + } else if (endRange) { rangeVersion = latestVersion; if (inserted) { - val = {nullptr, -1}; // Sentinel for "no point mutation here" + val = {nullptr, -1}; + pointVersion = -1; // Sentinel for "no point mutation here" +#ifndef NDEBUG + // If we inserted this, there would be adjacent clears and so the + // range would not be canonical Finger copy; finger.copyTo(copy); move(copy, latestVersion); - if (copy.searchPathSize() == 0) { - pointVersion = -1; // Sentinel for "no mutation ending here" - } else { - pointVersion = mm.base[copy.backNode()].entry->rangeVersion; - } + assert(copy.searchPathSize() == 0 || + mm.base[copy.backNode()].entry->rangeVersion < 0); +#endif } else { auto *entry = mm.base[finger.backNode()].entry; val = {entry->getVal(), entry->valLen}; pointVersion = entry->pointVersion; } + } else { + // Beginning of a clear range + pointVersion = -1; // Sentinel for "no point mutation here" + if (inserted) { + rangeVersion = -1; // Sentinel for "no mutation ending here" +#ifndef NDEBUG + // If there were a clear range here, it wouldn't be canonical + Finger copy; + finger.copyTo(copy); + move(copy, latestVersion); + assert(copy.searchPathSize() == 0 || + mm.base[copy.backNode()].entry->rangeVersion < 0); +#endif + } else { + auto *entry = mm.base[finger.backNode()].entry; + rangeVersion = entry->rangeVersion; + } + val = {nullptr, -1}; } + // TODO check for noop? + // Prepare new node const uint32_t node = newNode( pointVersion, rangeVersion, key.p, key.len, val->p, val->len, @@ -885,6 +917,8 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { // TODO tune? scanAndRemoveOldEntries(2 * numMutations + 10); + Arena arena; + // TODO Improve ILP? for (int i = 0; i < numMutations; ++i) { const auto &m = mutations[i]; @@ -893,7 +927,8 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { case Set: { search({m.param1, m.param1Len}, latestRoot, latestVersion, iter); - insert({m.param1, m.param1Len}, {{m.param2, m.param2Len}}, iter); + insert({m.param1, m.param1Len}, {{m.param2, m.param2Len}}, + /*endRange*/ false, iter); } break; case Clear: { // TODO we can avoid some insertions here. Complexity is getting out of @@ -923,17 +958,23 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { } if (engulfLeft && engulfRight) { - insert({next->getKey(), next->keyLen}, {}, copy); + insert({next->getKey(), next->keyLen}, {}, /*endRange*/ true, copy); if (found) { remove(iter); } + } else if (engulfLeft) { + assert(found); + remove(iter); + insert(keyAfter({m.param1, m.param1Len}, arena), {}, + /*endRange*/ true, iter); } else { - insert({m.param1, m.param1Len}, {{nullptr, -1}}, iter); + insert({m.param1, m.param1Len}, {{nullptr, -1}}, /*endRange*/ false, + iter); } } else { search({m.param1, m.param1Len}, latestRoot, latestVersion, iter); - insert({m.param1, m.param1Len}, {{nullptr, -1}}, iter); + insert({m.param1, m.param1Len}, {}, /*endRange*/ false, iter); // Check if we can engulf on the left { @@ -953,26 +994,26 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { // point search({m.param2, m.param2Len}, latestRoot, latestVersion, iter); - insert({m.param2, m.param2Len}, {}, iter); // Check if we can engulf on the right { - const auto *entry = mm.base[iter.backNode()].entry; - move(iter, latestVersion); - const auto *next = iter.searchPathSize() > 0 - ? mm.base[iter.backNode()].entry + Finger copy; + iter.copyTo(copy); + move(copy, latestVersion); + const auto *next = copy.searchPathSize() > 0 + ? mm.base[copy.backNode()].entry : nullptr; - if (entry->pointClear() && next && next->clearTo()) { - insert({next->getKey(), next->keyLen}, {}, iter); - move(iter, latestVersion); - remove(iter); + if (next && next->clearTo()) { + insert({next->getKey(), next->keyLen}, {}, /*endRange*/ true, + copy); + } else { + insert({m.param2, m.param2Len}, {}, /*endRange*/ true, iter); } } } } break; - default: // GCOVR_EXCL_LINE - assert(false); // GCOVR_EXCL_LINE - __builtin_unreachable(); // GCOVR_EXCL_LINE + default: // GCOVR_EXCL_LINE + assert(false); // GCOVR_EXCL_LINE } } roots.add(latestRoot, latestVersion); @@ -1154,18 +1195,17 @@ void materializeMutations(VersionedMap::Iterator::Impl *impl, const Entry *prev, impl->mutations[impl->mutationCount++] = { prev->getKey(), entry.getKey(), - prev->pointClear() ? prev->keyLen : prev->keyLen + 1, + prev->pointSet() ? prev->keyLen + 1 : prev->keyLen, entry.keyLen, VersionedMap::Clear, entry.rangeVersion}; } if (entry.pointMutation()) { if (entry.valLen < 0 /* pointClear */) { - if (next == nullptr || !next->clearTo()) { - impl->mutations[impl->mutationCount++] = { - entry.getKey(), nullptr, entry.keyLen, 0, - VersionedMap::Clear, entry.pointVersion}; - } + assert(next == nullptr || !next->clearTo()); + impl->mutations[impl->mutationCount++] = { + entry.getKey(), nullptr, entry.keyLen, 0, + VersionedMap::Clear, entry.pointVersion}; } else { impl->mutations[impl->mutationCount++] = { entry.getKey(), entry.getVal(), entry.keyLen, @@ -1534,16 +1574,22 @@ inline printf(" "); } printf("node %u: ", node); - printf("%.*s", mm.base[node].entry->keyLen, mm.base[node].entry->getKey()); - if (mm.base[node].entry->valLen >= 0) { - printf(" -> '%.*s' @ %" PRId64, mm.base[node].entry->valLen, - mm.base[node].entry->getVal(), mm.base[node].entry->pointVersion); - } else { + printBinary({mm.base[node].entry->getKey(), mm.base[node].entry->keyLen}); + if (mm.base[node].entry->pointSet()) { + printf(" -> '"); + printBinary({mm.base[node].entry->getVal(), mm.base[node].entry->valLen}); + printf("' @ %" PRId64, mm.base[node].entry->pointVersion); + } + if (mm.base[node].entry->pointClear()) { printf(" ", mm.base[node].entry->pointVersion); } if (mm.base[node].entry->clearTo()) { printf(" ", mm.base[node].entry->rangeVersion); } + if (mm.base[node].entry->pointVersion < 0 && + mm.base[node].entry->rangeVersion < 0) { + printf(" "); + } printf("\n"); VersionedMap::Impl::printInOrderHelper( version, child(node, false, version), @@ -1571,8 +1617,6 @@ struct __attribute__((visibility("default"))) PeakPrinter { #ifdef ENABLE_MAIN #include -#include "PrintMutation.h" - void breakpoint_me() {} int main() {