From be5f1b67c80002492799f4ac3cc9499c80ba4b72 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 5 Mar 2024 16:50:53 -0800 Subject: [PATCH] Interface change! addWrites now takes a single write version --- Bench.cpp | 28 +++++------------- ConflictSet.cpp | 53 +++++++++++++++-------------------- HashTable.cpp | 17 ++++++----- Internal.h | 9 +++--- RealDataBench.cpp | 3 +- SkipList.cpp | 21 +++++++------- conflict_set_c_api_test.c | 3 +- conflict_set_cxx_api_test.cpp | 3 +- include/ConflictSet.h | 27 +++++++++--------- symbols.txt | 2 +- 10 files changed, 71 insertions(+), 95 deletions(-) diff --git a/Bench.cpp b/Bench.cpp index 3ca08fb..6e4fe02 100644 --- a/Bench.cpp +++ b/Bench.cpp @@ -2,7 +2,6 @@ #include "Internal.h" #include #include -#include #if SHOW_MEMORY void showMemory(const ConflictSet &cs); @@ -78,10 +77,9 @@ void benchConflictSet() { w.begin.len = r.begin.len; w.end.p = r.end.p; w.end.len = 0; - w.writeVersion = version + 1; writes.push_back(w); } - cs.addWrites(writes.data(), writes.size()); + cs.addWrites(writes.data(), writes.size(), version + 1); ++version; } @@ -112,11 +110,10 @@ void benchConflictSet() { w.begin.len = begin.size(); w.end.p = end.data(); w.end.len = end.size(); - w.writeVersion = version + 1; writes.push_back(w); } + cs.addWrites(writes.data(), kOpsPerTx, version + 1); ++version; - cs.addWrites(writes.data(), kOpsPerTx); } { @@ -189,16 +186,12 @@ void benchConflictSet() { while (version < kMvccWindow) { auto v = ++version; - writes[0].writeVersion = v; - cs.addWrites(writes.data(), 1); + cs.addWrites(writes.data(), 1, v); } bench.run("point writes", [&]() { auto v = ++version; - for (auto &w : writes) { - w.writeVersion = v; - } - cs.addWrites(writes.data(), writes.size()); + cs.addWrites(writes.data(), writes.size(), v); cs.setOldestVersion(version - kMvccWindow); }); } @@ -219,10 +212,7 @@ void benchConflictSet() { bench.run("prefix writes", [&]() { auto v = ++version; - for (auto &w : writes) { - w.writeVersion = v; - } - cs.addWrites(writes.data(), writes.size()); + cs.addWrites(writes.data(), writes.size(), v); cs.setOldestVersion(version - kMvccWindow); }); } @@ -243,10 +233,7 @@ void benchConflictSet() { bench.run("range writes", [&]() { auto v = ++version; - for (auto &w : writes) { - w.writeVersion = v; - } - cs.addWrites(writes.data(), writes.size()); + cs.addWrites(writes.data(), writes.size(), v); cs.setOldestVersion(version - kMvccWindow); }); } @@ -263,12 +250,11 @@ void benchConflictSet() { auto x = __builtin_bswap64(version); memcpy(b, &x, 8); - w.writeVersion = v; w.begin.p = b; w.begin.len = 8; w.end.len = 0; w.end.p = b; - cs.addWrites(&w, 1); + cs.addWrites(&w, 1, v); cs.setOldestVersion(version - kMvccWindow); }); } diff --git a/ConflictSet.cpp b/ConflictSet.cpp index 3744936..0c4d81b 100644 --- a/ConflictSet.cpp +++ b/ConflictSet.cpp @@ -1847,18 +1847,18 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl { } } - void addWrites(const WriteRange *writes, int count) { + void addWrites(const WriteRange *writes, int count, int64_t writeVersion) { for (int i = 0; i < count; ++i) { const auto &w = writes[i]; auto begin = std::span(w.begin.p, w.begin.len); auto end = std::span(w.end.p, w.end.len); if (w.end.len > 0) { keyUpdates += 3; - addWriteRange(root, oldestVersion, begin, end, w.writeVersion, + addWriteRange(root, oldestVersion, begin, end, writeVersion, &allocators, this); } else { keyUpdates += 2; - addPointWrite(root, oldestVersion, begin, w.writeVersion, &allocators, + addPointWrite(root, oldestVersion, begin, writeVersion, &allocators, this); } } @@ -1954,8 +1954,9 @@ void ConflictSet::check(const ReadRange *reads, Result *results, return impl->check(reads, results, count); } -void ConflictSet::addWrites(const WriteRange *writes, int count) { - return impl->addWrites(writes, count); +void ConflictSet::addWrites(const WriteRange *writes, int count, + int64_t writeVersion) { + return impl->addWrites(writes, count, writeVersion); } void ConflictSet::setOldestVersion(int64_t oldestVersion) { @@ -2009,9 +2010,9 @@ ConflictSet_check(void *cs, const ConflictSet_ReadRange *reads, ((ConflictSet::Impl *)cs)->check(reads, results, count); } __attribute__((__visibility__("default"))) void -ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes, - int count) { - ((ConflictSet::Impl *)cs)->addWrites(writes, count); +ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes, int count, + int64_t writeVersion) { + ((ConflictSet::Impl *)cs)->addWrites(writes, count, writeVersion); } __attribute__((__visibility__("default"))) void ConflictSet_setOldestVersion(void *cs, int64_t oldestVersion) { @@ -2239,29 +2240,19 @@ void printTree() { ConflictSet::Impl cs{writeVersion}; ReferenceImpl refImpl{writeVersion}; Arena arena; - constexpr int kNumKeys = 4; - auto *write = new (arena) ConflictSet::WriteRange[kNumKeys]; - write[0].begin = "and"_s; - write[0].end = "ant"_s; - write[0].writeVersion = 1; - - write[1].begin = "any"_s; - write[1].end.len = 0; - write[1].writeVersion = 2; - - write[2].begin = "are"_s; - write[2].end.len = 0; - write[2].writeVersion = 3; - - write[3].begin = "art"_s; - write[3].end.len = 0; - write[3].writeVersion = 4; - - cs.addWrites(write, kNumKeys); - for (int i = 0; i < kNumKeys; ++i) { - write[i].writeVersion = ++writeVersion; - } - cs.addWrites(write, kNumKeys); + ConflictSet::WriteRange write; + write.begin = "and"_s; + write.end = "ant"_s; + cs.addWrites(&write, 1, ++writeVersion); + write.begin = "any"_s; + write.end = ""_s; + cs.addWrites(&write, 1, ++writeVersion); + write.begin = "are"_s; + write.end = ""_s; + cs.addWrites(&write, 1, ++writeVersion); + write.begin = "art"_s; + write.end = ""_s; + cs.addWrites(&write, 1, ++writeVersion); debugPrintDot(stdout, cs.root, &cs); } diff --git a/HashTable.cpp b/HashTable.cpp index 2886c4c..20f326d 100644 --- a/HashTable.cpp +++ b/HashTable.cpp @@ -36,11 +36,13 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl { } } - void addWrites(const ConflictSet::WriteRange *writes, int count) { + void addWrites(const ConflictSet::WriteRange *writes, int count, + int64_t writeVersion) { for (int i = 0; i < count; ++i) { auto &max = map[std::string((const char *)writes[i].begin.p, writes[i].begin.len)]; - max = std::max(max, writes[i].writeVersion); + assert(writeVersion >= max); + max = writeVersion; keyUpdates += 2; } } @@ -83,8 +85,9 @@ void ConflictSet::check(const ReadRange *reads, Result *results, return impl->check(reads, results, count); } -void ConflictSet::addWrites(const WriteRange *writes, int count) { - return impl->addWrites(writes, count); +void ConflictSet::addWrites(const WriteRange *writes, int count, + int64_t writeVersion) { + return impl->addWrites(writes, count, writeVersion); } void ConflictSet::setOldestVersion(int64_t oldestVersion) { @@ -121,9 +124,9 @@ ConflictSet_check(void *cs, const ConflictSet_ReadRange *reads, ((ConflictSet::Impl *)cs)->check(reads, results, count); } __attribute__((__visibility__("default"))) void -ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes, - int count) { - ((ConflictSet::Impl *)cs)->addWrites(writes, count); +ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes, int count, + int64_t writeVersion) { + ((ConflictSet::Impl *)cs)->addWrites(writes, count, writeVersion); } __attribute__((__visibility__("default"))) void ConflictSet_setOldestVersion(void *cs, int64_t oldestVersion) { diff --git a/Internal.h b/Internal.h index b01138e..0cedb91 100644 --- a/Internal.h +++ b/Internal.h @@ -404,7 +404,8 @@ struct ReferenceImpl { : ConflictSet::Commit; } } - void addWrites(const ConflictSet::WriteRange *writes, int count) { + void addWrites(const ConflictSet::WriteRange *writes, int count, + int64_t writeVersion) { for (int i = 0; i < count; ++i) { auto begin = std::string((const char *)writes[i].begin.p, writes[i].begin.len); @@ -412,7 +413,6 @@ struct ReferenceImpl { writes[i].end.len == 0 ? begin + std::string("\x00", 1) : std::string((const char *)writes[i].end.p, writes[i].end.len); - auto writeVersion = writes[i].writeVersion; auto prevVersion = (--writeVersionMap.upper_bound(end))->second; for (auto iter = writeVersionMap.lower_bound(begin), endIter = writeVersionMap.lower_bound(end); @@ -547,7 +547,6 @@ template struct TestDriver { ++iter; --rangesRemaining; } - writes[i].writeVersion = v; #if DEBUG_VERBOSE && !defined(NDEBUG) if (writes[i].end.len == 0) { fprintf(stderr, "Write: {%s} -> %d\n", @@ -565,10 +564,10 @@ template struct TestDriver { assert(i == numPointWrites + numRangeWrites); CALLGRIND_START_INSTRUMENTATION; - cs.addWrites(writes, numPointWrites + numRangeWrites); + cs.addWrites(writes, numPointWrites + numRangeWrites, v); CALLGRIND_STOP_INSTRUMENTATION; - refImpl.addWrites(writes, numPointWrites + numRangeWrites); + refImpl.addWrites(writes, numPointWrites + numRangeWrites, v); oldestVersion = std::max(writeVersion - arbitrary.bounded(10), oldestVersion); diff --git a/RealDataBench.cpp b/RealDataBench.cpp index acdc564..04c7031 100644 --- a/RealDataBench.cpp +++ b/RealDataBench.cpp @@ -96,7 +96,6 @@ int main(int argc, const char **argv) { // Add unconditionally so that the load doesn't actually depend on the // conflict rate ConflictSet::WriteRange w; - w.writeVersion = ++version; w.begin.p = (const uint8_t *)write.data(); w.begin.len = write.size(); w.end.len = 0; @@ -104,7 +103,7 @@ int main(int argc, const char **argv) { addBytes += write.size(); timer = now(); - cs.addWrites(&w, 1); + cs.addWrites(&w, 1, ++version); addTime += now() - timer; write = {}; diff --git a/SkipList.cpp b/SkipList.cpp index fdacce3..cd0d6dc 100644 --- a/SkipList.cpp +++ b/SkipList.cpp @@ -286,7 +286,7 @@ public: void swap(SkipList &other) { std::swap(header, other.header); } void addConflictRanges(const Finger *fingers, int rangeCount, - Version *version) { + Version version) { for (int r = rangeCount - 1; r >= 0; r--) { const Finger &startF = fingers[r * 2]; const Finger &endF = fingers[r * 2 + 1]; @@ -295,7 +295,7 @@ public: insert(endF, endF.finger[0]->getMaxVersion(0)); remove(startF, endF); - insert(startF, version[r]); + insert(startF, version); } } @@ -588,7 +588,8 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl { } } - void addWrites(const ConflictSet::WriteRange *writes, int count) { + void addWrites(const ConflictSet::WriteRange *writes, int count, + int64_t writeVersion) { Arena arena; const int stringCount = count * 2; @@ -606,11 +607,10 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl { values[i * 2 + 1] = w.end.len > 0 ? StringRef{w.end.p, size_t(w.end.len)} : keyAfter(arena, values[i * 2]); - writeVersions[i] = w.writeVersion; keyUpdates += 2; } skipList.find(values, fingers, temp, ss); - skipList.addConflictRanges(fingers, ss / 2, writeVersions); + skipList.addConflictRanges(fingers, ss / 2, writeVersion); ss = stripeSize; } } @@ -640,8 +640,9 @@ void ConflictSet::check(const ReadRange *reads, Result *results, return impl->check(reads, results, count); } -void ConflictSet::addWrites(const WriteRange *writes, int count) { - return impl->addWrites(writes, count); +void ConflictSet::addWrites(const WriteRange *writes, int count, + int64_t writeVersion) { + return impl->addWrites(writes, count, writeVersion); } void ConflictSet::setOldestVersion(int64_t oldestVersion) { @@ -678,9 +679,9 @@ ConflictSet_check(void *cs, const ConflictSet_ReadRange *reads, ((ConflictSet::Impl *)cs)->check(reads, results, count); } __attribute__((__visibility__("default"))) void -ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes, - int count) { - ((ConflictSet::Impl *)cs)->addWrites(writes, count); +ConflictSet_addWrites(void *cs, const ConflictSet_WriteRange *writes, int count, + int64_t writeVersion) { + ((ConflictSet::Impl *)cs)->addWrites(writes, count, writeVersion); } __attribute__((__visibility__("default"))) void ConflictSet_setOldestVersion(void *cs, int64_t oldestVersion) { diff --git a/conflict_set_c_api_test.c b/conflict_set_c_api_test.c index d593b59..ed60264 100644 --- a/conflict_set_c_api_test.c +++ b/conflict_set_c_api_test.c @@ -10,8 +10,7 @@ int main(void) { w.begin.p = (const uint8_t *)"0000"; w.begin.len = 4; w.end.len = 0; - w.writeVersion = 1; - ConflictSet_addWrites(cs, &w, 1); + ConflictSet_addWrites(cs, &w, 1, 1); r.begin.p = (const uint8_t *)"0000"; r.begin.len = 4; r.end.len = 0; diff --git a/conflict_set_cxx_api_test.cpp b/conflict_set_cxx_api_test.cpp index b057dbf..725b233 100644 --- a/conflict_set_cxx_api_test.cpp +++ b/conflict_set_cxx_api_test.cpp @@ -8,8 +8,7 @@ int main(void) { w.begin.p = (const uint8_t *)"0000"; w.begin.len = 4; w.end.len = 0; - w.writeVersion = 1; - cs.addWrites(&w, 1); + cs.addWrites(&w, 1, 1); ConflictSet::Result result; ConflictSet::ReadRange r; r.begin.p = (const uint8_t *)"0000"; diff --git a/include/ConflictSet.h b/include/ConflictSet.h index a8e6ebd..9f57635 100644 --- a/include/ConflictSet.h +++ b/include/ConflictSet.h @@ -44,22 +44,21 @@ struct __attribute__((__visibility__("default"))) ConflictSet { Key end; int64_t readVersion; }; - /** Denotes a set of keys to be considered written at `writeVersion` */ + /** Denotes a set of keys to be written at `writeVersion` */ struct WriteRange { Key begin; /** `end` having length 0 denotes that this range is the single key {begin}. * Otherwise this denotes the range [begin, end) */ Key end; - /** Write version must be greater than all write versions in all previous - * calls to `addWrites` */ - int64_t writeVersion; }; /** The result of checking reads[i] is written in results[i]. */ void check(const ReadRange *reads, Result *results, int count) const; /** `writes` must be sorted ascending, and must not have adjacent or * overlapping ranges. Reads intersecting writes where readVersion < - * writeVersion will result in `Conflict` (or `TooOld`, eventually) */ - void addWrites(const WriteRange *writes, int count); + * `writeVersion` will result in `Conflict` (or `TooOld`, eventually). + * `writeVersion must be greater than all write versions in all previous + * calls to `addWrites` */ + void addWrites(const WriteRange *writes, int count, int64_t writeVersion); /** Reads where readVersion < oldestVersion will result in `TooOld`. Must be * greater than any previous oldestVersion. */ void setOldestVersion(int64_t oldestVersion); @@ -110,25 +109,25 @@ typedef struct { ConflictSet_Key end; int64_t readVersion; } ConflictSet_ReadRange; -/** Denotes a set of keys to be considered written at `writeVersion` */ +/** Denotes a set of keys to be written at `writeVersion` */ typedef struct { ConflictSet_Key begin; /** `end` having length 0 denotes that this range is the single key {begin}. * Otherwise this denotes the range [begin, end) */ ConflictSet_Key end; - /** Write version must be greater than all write versions in all previous - * calls to `addWrites` */ - int64_t writeVersion; } ConflictSet_WriteRange; /** The result of checking reads[i] is written in results[i]. */ void ConflictSet_check(ConflictSet *cs, const ConflictSet_ReadRange *reads, ConflictSet_Result *results, int count); -/** `writes` must be sorted ascending, and must not have adjacent or overlapping - * ranges. Reads intersecting writes where readVersion < writeVersion will - * result in `Conflict` (or `TooOld`, eventually) */ +/** `writes` must be sorted ascending, and must not have adjacent or + * overlapping ranges. Reads intersecting writes where readVersion < + * `writeVersion` will result in `Conflict` (or `TooOld`, eventually). + * `writeVersion must be greater than all write versions in all previous + * calls to `addWrites` */ void ConflictSet_addWrites(ConflictSet *cs, - const ConflictSet_WriteRange *writes, int count); + const ConflictSet_WriteRange *writes, int count, + int64_t writeVersion); /** Reads where readVersion < oldestVersion will result in `TooOld`. Must be * greater than any previous oldestVersion. */ void ConflictSet_setOldestVersion(ConflictSet *cs, int64_t oldestVersion); diff --git a/symbols.txt b/symbols.txt index 06ea86e..b34b0a7 100644 --- a/symbols.txt +++ b/symbols.txt @@ -4,7 +4,7 @@ ConflictSet_create ConflictSet_destroy ConflictSet_setOldestVersion _ZN11ConflictSet16setOldestVersionEl -_ZN11ConflictSet9addWritesEPKNS_10WriteRangeEi +_ZN11ConflictSet9addWritesEPKNS_10WriteRangeEil _ZN11ConflictSetaSEOS_ _ZN11ConflictSetC1El _ZN11ConflictSetC1EOS_