From f1f4d6667833944d2da97a6e8cab0dd8bd56470e Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 28 May 2024 21:38:08 -0700 Subject: [PATCH] Prepare to fully canonicalize views --- ApiTest.cpp | 4 ++-- Facade.h | 4 ++-- FdbVersionedMap.cpp | 2 +- PrintMutation.h | 4 ++-- VersionedMap.cpp | 11 ++++++++--- include/VersionedMap.h | 10 +++++----- 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/ApiTest.cpp b/ApiTest.cpp index c04b8e0..2123aad 100644 --- a/ApiTest.cpp +++ b/ApiTest.cpp @@ -54,7 +54,7 @@ void setAndClearPrev() { auto m = *iter++; int64_t k = __builtin_bswap64(version); - assert(m.version == version); + assert(m.notModifiedSince == version); assert(m.param1Len == 8); assert(m.param2Len == 8); int64_t zero = __builtin_bswap64(0); @@ -63,7 +63,7 @@ void setAndClearPrev() { assert(m.type == weaselab::VersionedMap::Clear); m = *iter++; - assert(m.version == version); + assert(m.notModifiedSince == version); assert(m.param1Len == 8); assert(m.param2Len == 8); assert(memcmp(m.param1, &k, 8) == 0); diff --git a/Facade.h b/Facade.h index d9c7a25..1f9f632 100644 --- a/Facade.h +++ b/Facade.h @@ -191,8 +191,8 @@ struct Facade { for (auto iter = versioned.begin(version), end = versioned.end(version); iter != end; ++iter) { auto m = *iter; - assert(m.version <= version); - if (m.version <= unversionedVersion) { + assert(m.notModifiedSince <= version); + if (m.notModifiedSince <= unversionedVersion) { continue; } switch (m.type) { diff --git a/FdbVersionedMap.cpp b/FdbVersionedMap.cpp index 5ea8940..e7ff1ef 100644 --- a/FdbVersionedMap.cpp +++ b/FdbVersionedMap.cpp @@ -1345,7 +1345,7 @@ VersionedMap::Iterator::operator*() const { result.param2 = impl->iter->getValue().p; result.param2Len = impl->iter->getValue().len; } - result.version = impl->iter.insertVersion(); + result.notModifiedSince = impl->iter.insertVersion(); return result; } diff --git a/PrintMutation.h b/PrintMutation.h index ec39215..9f6164a 100644 --- a/PrintMutation.h +++ b/PrintMutation.h @@ -27,7 +27,7 @@ printMutation(const weaselab::VersionedMap::Iterator::VersionedMutation &m) { printf("x%02x", c); } } - printf("' @ %" PRId64 "\n", m.version); + printf("' @ %" PRId64 "\n", m.notModifiedSince); break; case weaselab::VersionedMap::Clear: printf("clear ["); @@ -48,7 +48,7 @@ printMutation(const weaselab::VersionedMap::Iterator::VersionedMutation &m) { printf("x%02x", c); } } - printf(") @ %" PRId64 "\n", m.version); + printf(") @ %" PRId64 "\n", m.notModifiedSince); break; default: // GCOVR_EXCL_LINE __builtin_unreachable(); // GCOVR_EXCL_LINE diff --git a/VersionedMap.cpp b/VersionedMap.cpp index ce0e7d9..b97804d 100644 --- a/VersionedMap.cpp +++ b/VersionedMap.cpp @@ -615,10 +615,9 @@ 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`. - // `finger` becomes the search path of `key` + // `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) { - - search(key, latestRoot, latestVersion, finger); const bool inserted = finger.backNode() == 0; int64_t pointVersion, rangeVersion; @@ -878,9 +877,13 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { Finger iter; switch (m.type) { case Set: { + search({m.param1, m.param1Len}, latestRoot, + latestVersion, iter); insert({m.param1, m.param1Len}, {{m.param2, m.param2Len}}, iter); } break; case Clear: { + search({m.param1, m.param1Len}, latestRoot, + latestVersion, iter); insert({m.param1, m.param1Len}, {{nullptr, -1}}, iter); if (m.param2Len > 0) { move(iter, latestVersion); @@ -891,6 +894,8 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl { } // TODO reuse finger? It should be one rank away from its insertion // point + search({m.param2, m.param2Len}, latestRoot, + latestVersion, iter); insert({m.param2, m.param2Len}, {}, iter); } } break; diff --git a/include/VersionedMap.h b/include/VersionedMap.h index 8f97691..82710fd 100644 --- a/include/VersionedMap.h +++ b/include/VersionedMap.h @@ -86,16 +86,13 @@ struct __attribute__((__visibility__("default"))) VersionedMap { /** The version of the most recent call to `setOldestVersion`. */ int64_t getOldestVersion() const; - /** Iterates through a canonicalized[1] view of all the mutations + /** Iterates through a canonicalized view of all the mutations * from `oldestVersion` to the iterator's version. There may be mutations from * versions < `oldestVersion`, but they won't affect the result, and can be * ignored if desired. It's thread-safe to operate on an iterator concurrently * with any method of `VersionedMap`, as long as it's not invalidated by * `setOldestVersion`. * @warning must not outlive its `VersionedMap`. - * - * [1]: Clears at different versions may be adjacent. This is necessary for - * precisely tracking at what version the mutations take effect. */ struct Iterator { @@ -114,7 +111,10 @@ struct __attribute__((__visibility__("default"))) VersionedMap { int param1Len; int param2Len; MutationType type; - int64_t version; + /** The set of keys modified by this mutation have not been modified since + * this version. They were not necessarily modified at this version + * though. */ + int64_t notModifiedSince; }; /** iter must not be `end()`. Memory pointed-to by return value is valid as