Bound memory, and disable free list for now

CC #9

.clangd

@@ -1,2 +1,2 @@
 CompileFlags:
-  Add: [-DENABLE_MAIN, -UNDEBUG, -DENABLE_FUZZ, -DTHREAD_TEST, -fexceptions]
+  Add: [-DENABLE_MAIN, -UNDEBUG, -DENABLE_FUZZ, -fexceptions]

Bench.cpp

@@ -34,7 +34,7 @@ ConflictSet::ReadRange singleton(Arena &arena, std::span<const uint8_t> key) {
       std::span<uint8_t>(new (arena) uint8_t[key.size() + 1], key.size() + 1);
   memcpy(r.data(), key.data(), key.size());
   r[key.size()] = 0;
-  return {{key.data(), int(key.size())}, {r.data(), int(r.size())}, 0};
+  return {key.data(), int(key.size()), r.data(), int(r.size())};
 }
 
 ConflictSet::ReadRange prefixRange(Arena &arena, std::span<const uint8_t> key) {
@@ -52,7 +52,7 @@ ConflictSet::ReadRange prefixRange(Arena &arena, std::span<const uint8_t> key) {
   auto r = std::span<uint8_t>(new (arena) uint8_t[index + 1], index + 1);
   memcpy(r.data(), key.data(), index + 1);
   r[r.size() - 1]++;
-  return {{key.data(), int(key.size())}, {r.data(), int(r.size())}, 0};
+  return {key.data(), int(key.size()), r.data(), int(r.size())};
 }
 
 void benchConflictSet() {
@@ -258,4 +258,4 @@ void benchConflictSet() {
   }
 }
 
-int main(void) { benchConflictSet(); }
+int main(void) { benchConflictSet(); }

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.18)
 project(
-  conflict-set
+  conflict_set
   VERSION 0.0.1
   DESCRIPTION
     "A data structure for optimistic concurrency control on ranges of bitwise-lexicographically-ordered keys."
@@ -22,11 +22,7 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
                                                "MinSizeRel" "RelWithDebInfo")
 endif()
 
-add_compile_options(-fdata-sections -ffunction-sections -Wswitch-enum
-                    -Werror=switch-enum)
-
-option(USE_SIMD_FALLBACK
-       "Use fallback implementations of functions that use SIMD" OFF)
+add_compile_options(-fdata-sections -ffunction-sections)
 
 # This is encouraged according to
 # https://valgrind.org/docs/manual/manual-core-adv.html#manual-core-adv.clientreq
@@ -46,20 +42,18 @@ endif()
 include(CheckIncludeFileCXX)
 include(CMakePushCheckState)
 
-if(NOT USE_SIMD_FALLBACK)
-  cmake_push_check_state()
-  list(APPEND CMAKE_REQUIRED_FLAGS -mavx)
-  check_include_file_cxx("immintrin.h" HAS_AVX)
-  if(HAS_AVX)
-    add_compile_options(-mavx)
-    add_compile_definitions(HAS_AVX)
-  endif()
-  cmake_pop_check_state()
+cmake_push_check_state()
+list(APPEND CMAKE_REQUIRED_FLAGS -mavx)
+check_include_file_cxx("immintrin.h" HAS_AVX)
+if(HAS_AVX)
+  add_compile_options(-mavx)
+  add_compile_definitions(HAS_AVX)
+endif()
+cmake_pop_check_state()
 
-  check_include_file_cxx("arm_neon.h" HAS_ARM_NEON)
-  if(HAS_ARM_NEON)
-    add_compile_definitions(HAS_ARM_NEON)
-  endif()
+check_include_file_cxx("arm_neon.h" HAS_ARM_NEON)
+if(HAS_ARM_NEON)
+  add_compile_definitions(HAS_ARM_NEON)
 endif()
 
 set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "")
@@ -83,19 +77,19 @@ if(NOT APPLE)
                       LINKER:--version-script=${CMAKE_SOURCE_DIR}/linker.map)
 endif()
 
-add_library(${PROJECT_NAME}-static STATIC
+add_library(${PROJECT_NAME}_static STATIC
             $<TARGET_OBJECTS:${PROJECT_NAME}_object>)
 if(NOT CMAKE_BUILD_TYPE STREQUAL Debug)
-  set_target_properties(${PROJECT_NAME}-static PROPERTIES LINKER_LANGUAGE C)
+  set_target_properties(${PROJECT_NAME}_static PROPERTIES LINKER_LANGUAGE C)
 endif()
 
 if(NOT APPLE AND CMAKE_OBJCOPY)
   add_custom_command(
-    TARGET conflict-set-static
+    TARGET conflict_set_static
     POST_BUILD
     COMMAND
       ${CMAKE_OBJCOPY} --keep-global-symbols=${CMAKE_SOURCE_DIR}/symbols.txt
-      $<TARGET_FILE:${PROJECT_NAME}-static>)
+      $<TARGET_FILE:${PROJECT_NAME}_static>)
 endif()
 
 set(TEST_FLAGS -Wall -Wextra -Wunreachable-code -Wpedantic -UNDEBUG)
@@ -160,10 +154,6 @@ if(BUILD_TESTING)
 
   add_executable(fuzz_driver ConflictSet.cpp FuzzTestDriver.cpp)
   target_compile_options(fuzz_driver PRIVATE ${TEST_FLAGS})
-  if(NOT CMAKE_CROSSCOMPILING)
-    target_compile_options(fuzz_driver PRIVATE -fsanitize=address,undefined)
-    target_link_options(fuzz_driver PRIVATE -fsanitize=address,undefined)
-  endif()
   target_compile_definitions(fuzz_driver PRIVATE ENABLE_FUZZ)
   target_include_directories(fuzz_driver
                              PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include)
@@ -172,35 +162,10 @@ if(BUILD_TESTING)
     add_test(NAME conflict_set_fuzz_${hash} COMMAND fuzz_driver ${TEST})
   endforeach()
 
-  # tsan
-
-  if(NOT CMAKE_CROSSCOMPILING AND NOT CMAKE_BUILD_TYPE STREQUAL Debug)
-    add_executable(tsan_driver ConflictSet.cpp FuzzTestDriver.cpp)
-    target_compile_options(tsan_driver PRIVATE ${TEST_FLAGS} -fsanitize=thread)
-    target_link_options(tsan_driver PRIVATE -fsanitize=thread)
-    target_compile_definitions(tsan_driver PRIVATE ENABLE_FUZZ THREAD_TEST)
-    target_include_directories(tsan_driver
-                               PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include)
-    foreach(TEST ${CORPUS_TESTS})
-      get_filename_component(hash ${TEST} NAME)
-      add_test(NAME conflict_set_tsan_${hash} COMMAND tsan_driver ${TEST})
-    endforeach()
-  endif()
-
   add_executable(driver TestDriver.cpp)
   target_compile_options(driver PRIVATE ${TEST_FLAGS})
   target_link_libraries(driver PRIVATE ${PROJECT_NAME})
 
-  add_executable(script_test ScriptTest.cpp)
-  target_compile_options(script_test PRIVATE ${TEST_FLAGS})
-  target_link_libraries(script_test PRIVATE ${PROJECT_NAME})
-
-  file(GLOB SCRIPT_TESTS ${CMAKE_SOURCE_DIR}/script_tests/*)
-  foreach(TEST ${SCRIPT_TESTS})
-    get_filename_component(name ${TEST} NAME)
-    add_test(NAME conflict_set_script_${name} COMMAND script_test ${TEST})
-  endforeach()
-
   add_executable(driver_skip_list TestDriver.cpp)
   target_compile_options(driver_skip_list PRIVATE ${TEST_FLAGS})
   target_link_libraries(driver_skip_list PRIVATE skip_list)
@@ -247,7 +212,7 @@ if(BUILD_TESTING)
       NAME conflict_set_static_symbols
       COMMAND
         ${CMAKE_SOURCE_DIR}/test_symbols.sh
-        $<TARGET_FILE:${PROJECT_NAME}-static> ${CMAKE_SOURCE_DIR}/symbols.txt)
+        $<TARGET_FILE:${PROJECT_NAME}_static> ${CMAKE_SOURCE_DIR}/symbols.txt)
   endif()
 
   # bench
@@ -272,10 +237,6 @@ set(CPACK_RESOURCE_FILE_README "${CMAKE_CURRENT_SOURCE_DIR}/README.md")
 set(CPACK_RPM_PACKAGE_ARCHITECTURE ${CMAKE_SYSTEM_PROCESSOR})
 set(CPACK_RPM_SPEC_INSTALL_POST "/bin/true") # avoid stripping
 set(CPACK_RPM_PACKAGE_LICENSE "Apache 2.0")
-set(CPACK_RPM_FILE_NAME RPM-DEFAULT)
-
-# deb
-set(CPACK_DEBIAN_FILE_NAME DEB-DEFAULT)
 
 include(CPack)
 
@@ -287,7 +248,7 @@ target_include_directories(
          $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}/${PROJECT_NAME}>)
 
 target_include_directories(
-  ${PROJECT_NAME}-static
+  ${PROJECT_NAME}_static
   PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
          $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}/${PROJECT_NAME}>)
 
@@ -296,13 +257,13 @@ set_target_properties(
                              SOVERSION ${PROJECT_VERSION_MAJOR})
 
 install(
-  TARGETS ${PROJECT_NAME} ${PROJECT_NAME}-static
-  EXPORT conflict-setConfig
+  TARGETS ${PROJECT_NAME} ${PROJECT_NAME}_static
+  EXPORT ConflictSetConfig
   ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
   LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
   RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})
 install(DIRECTORY include/
         DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/${PROJECT_NAME})
 
-install(EXPORT conflict-setConfig
-        DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/conflict-set/cmake)
+install(EXPORT ConflictSetConfig
+        DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/ConflictSet/cmake)

FuzzTestDriver.cpp

@@ -2,25 +2,15 @@
 #include <cstdint>
 #include <fstream>
 #include <sstream>
-#include <thread>
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
 
 int main(int argc, char **argv) {
-  auto doTest = [&]() {
-    for (int i = 1; i < argc; ++i) {
-      std::ifstream t(argv[i], std::ios::binary);
-      std::stringstream buffer;
-      buffer << t.rdbuf();
-      auto str = buffer.str();
-      LLVMFuzzerTestOneInput((const uint8_t *)str.data(), str.size());
-    }
-  };
-#ifdef THREAD_TEST
-  std::thread thread2{doTest};
-#endif
-  doTest();
-#ifdef THREAD_TEST
-  thread2.join();
-#endif
+  for (int i = 1; i < argc; ++i) {
+    std::ifstream t(argv[i], std::ios::binary);
+    std::stringstream buffer;
+    buffer << t.rdbuf();
+    auto str = buffer.str();
+    LLVMFuzzerTestOneInput((const uint8_t *)str.data(), str.size());
+  }
 }

HashTable.cpp

@@ -96,15 +96,13 @@ void ConflictSet::setOldestVersion(int64_t oldestVersion) {
   return impl->setOldestVersion(oldestVersion);
 }
 
-int64_t ConflictSet::getBytes() const { return -1; }
-
 ConflictSet::ConflictSet(int64_t oldestVersion)
     : impl(new (safe_malloc(sizeof(Impl))) Impl{oldestVersion}) {}
 
 ConflictSet::~ConflictSet() {
   if (impl) {
     impl->~Impl();
-    safe_free(impl, sizeof(Impl));
+    free(impl);
   }
 }
 
@@ -144,11 +142,6 @@ ConflictSet_create(int64_t oldestVersion) {
 __attribute__((__visibility__("default"))) void ConflictSet_destroy(void *cs) {
   using Impl = ConflictSet::Impl;
   ((Impl *)cs)->~Impl();
-  safe_free(cs, sizeof(Impl));
-}
-__attribute__((__visibility__("default"))) int64_t
-ConflictSet_getBytes(void *cs) {
-  using Impl = ConflictSet::Impl;
-  return -1;
+  free(cs);
 }
 }

Internal.h

@@ -2,8 +2,6 @@
 
 #include "ConflictSet.h"
 
-using namespace weaselab;
-
 #include <bit>
 #include <cassert>
 #include <compare>
@@ -12,12 +10,10 @@ using namespace weaselab;
 #include <cstdlib>
 #include <cstring>
 #include <inttypes.h>
-#include <latch>
 #include <map>
 #include <set>
 #include <span>
 #include <string>
-#include <thread>
 #include <unordered_set>
 #include <utility>
 #include <vector>
@@ -39,76 +35,16 @@ operator<=>(const std::span<const uint8_t> &lhs,
   return lhs.size() <=> rhs.size();
 }
 
-[[nodiscard]] inline auto operator<=>(const std::span<const uint8_t> &lhs,
-                                      const ConflictSet::Key &rhs) noexcept {
-  int cl = std::min<int>(lhs.size(), rhs.len);
-  if (cl > 0) {
-    if (auto c = memcmp(lhs.data(), rhs.p, cl) <=> 0; c != 0) {
-      return c;
-    }
-  }
-  return lhs.size() <=> size_t(rhs.len);
-}
-
 // 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.
 
 // GCOVR_EXCL_START
 
-#if SHOW_MEMORY
-inline int64_t mallocBytes = 0;
-inline int64_t peakMallocBytes = 0;
-#endif
-
-inline thread_local int64_t mallocBytesDelta = 0;
-
-#ifndef NDEBUG
-constexpr auto kMallocHeaderSize = 16;
-#endif
-
-// malloc that aborts on OOM and thus always returns a non-null pointer. Must be
-// paired with `safe_free`.
 __attribute__((always_inline)) inline void *safe_malloc(size_t s) {
-  mallocBytesDelta += s;
-#if SHOW_MEMORY
-  mallocBytes += s;
-  if (mallocBytes > peakMallocBytes) {
-    peakMallocBytes = mallocBytes;
+  if (void *p = malloc(s)) {
+    return p;
   }
-#endif
-  void *p = malloc(s
-#ifndef NDEBUG
-                   + kMallocHeaderSize
-#endif
-  );
-  if (p == nullptr) {
-    abort();
-  }
-#ifndef NDEBUG
-  memcpy(p, &s, sizeof(s));
-  (char *&)p += kMallocHeaderSize;
-#endif
-  return p;
-}
-
-// Must be paired with `safe_malloc`.
-//
-// There's nothing safer about this than free. Only called safe_free for
-// symmetry with safe_malloc.
-__attribute__((always_inline)) inline void safe_free(void *p, size_t s) {
-  mallocBytesDelta -= s;
-#if SHOW_MEMORY
-  mallocBytes -= s;
-  free(p);
-#else
-#ifndef NDEBUG
-  (char *&)p -= kMallocHeaderSize;
-  size_t expected;
-  memcpy(&expected, p, sizeof(expected));
-  assert(s == expected);
-#endif
-  free(p);
-#endif
+  abort();
 }
 
 // ==================== BEGIN ARENA IMPL ====================
@@ -193,7 +129,7 @@ inline Arena::Arena(int initialSize) : impl(nullptr) {
 inline void onDestroy(Arena::ArenaImpl *impl) {
   while (impl) {
     auto *prev = impl->prev;
-    safe_free(impl, sizeof(Arena::ArenaImpl) + impl->capacity);
+    free(impl);
     impl = prev;
   }
 }
@@ -413,6 +349,34 @@ inline uint32_t Arbitrary::bounded(uint32_t s) {
 
 // ==================== END ARBITRARY IMPL ====================
 
+// ==================== BEGIN UTILITIES IMPL ====================
+
+// Call Stepwise::step for each element of remaining until it returns true.
+// Applies a permutation to `remaining` as a side effect.
+template <class Stepwise> void runInterleaved(std::span<Stepwise> remaining) {
+  while (remaining.size() > 0) {
+    for (int i = 0; i < int(remaining.size());) {
+      bool done = remaining[i].step();
+      if (done) {
+        if (i != int(remaining.size()) - 1) {
+          using std::swap;
+          swap(remaining[i], remaining.back());
+        }
+        remaining = remaining.subspan(0, remaining.size() - 1);
+      } else {
+        ++i;
+      }
+    }
+  }
+};
+
+template <class Stepwise> void runSequential(std::span<Stepwise> remaining) {
+  for (auto &r : remaining) {
+    while (!r.step()) {
+    }
+  }
+}
+
 struct ReferenceImpl {
   explicit ReferenceImpl(int64_t oldestVersion) : oldestVersion(oldestVersion) {
     writeVersionMap[""] = oldestVersion;
@@ -506,18 +470,6 @@ inline std::string printable(std::span<const uint8_t> key) {
   return printable(std::string_view((const char *)key.data(), key.size()));
 }
 
-inline const char *resultToStr(ConflictSet::Result r) {
-  switch (r) {
-  case ConflictSet::Commit:
-    return "commit";
-  case ConflictSet::Conflict:
-    return "conflict";
-  case ConflictSet::TooOld:
-    return "too old";
-  }
-  abort();
-}
-
 namespace {
 
 template <class ConflictSetImpl> struct TestDriver {
@@ -530,10 +482,22 @@ template <class ConflictSetImpl> struct TestDriver {
   ConflictSetImpl cs{oldestVersion};
   ReferenceImpl refImpl{oldestVersion};
 
-  constexpr static auto kMaxKeyLen = 8;
+  constexpr static auto kMaxKeyLen = 64;
 
   bool ok = true;
 
+  static const char *resultToStr(ConflictSet::Result r) {
+    switch (r) {
+    case ConflictSet::Commit:
+      return "commit";
+    case ConflictSet::Conflict:
+      return "conflict";
+    case ConflictSet::TooOld:
+      return "too old";
+    }
+    abort();
+  }
+
   // Call until it returns true, for "done". Check internal invariants etc
   // between calls to next.
   bool next() {
@@ -668,60 +632,32 @@ template <class ConflictSetImpl> struct TestDriver {
       auto *results2 =
           new (arena) ConflictSet::Result[numPointReads + numRangeReads];
 
-#ifdef THREAD_TEST
-      auto *results3 =
-          new (arena) ConflictSet::Result[numPointReads + numRangeReads];
-      std::latch ready{1};
-      std::thread thread2{[&]() {
-        ready.count_down();
-        cs.check(reads, results3, numPointReads + numRangeReads);
-      }};
-      ready.wait();
-#endif
-
       CALLGRIND_START_INSTRUMENTATION;
       cs.check(reads, results1, numPointReads + numRangeReads);
       CALLGRIND_STOP_INSTRUMENTATION;
 
       refImpl.check(reads, results2, numPointReads + numRangeReads);
-
-      auto compareResults = [reads](ConflictSet::Result *results1,
-                                    ConflictSet::Result *results2, int count) {
-        for (int i = 0; i < count; ++i) {
-          if (results1[i] != results2[i]) {
-            if (reads[i].end.len == 0) {
-              fprintf(stderr,
-                      "Expected %s, got %s for read of {%s} at version %" PRId64
-                      "\n",
-                      resultToStr(results2[i]), resultToStr(results1[i]),
-                      printable(reads[i].begin).c_str(), reads[i].readVersion);
-            } else {
-              fprintf(
-                  stderr,
-                  "Expected %s, got %s for read of [%s, %s) at version %" PRId64
-                  "\n",
-                  resultToStr(results2[i]), resultToStr(results1[i]),
-                  printable(reads[i].begin).c_str(),
-                  printable(reads[i].end).c_str(), reads[i].readVersion);
-            }
-            return false;
+      for (int i = 0; i < numPointReads + numRangeReads; ++i) {
+        if (results1[i] != results2[i]) {
+          if (reads[i].end.len == 0) {
+            fprintf(stderr,
+                    "Expected %s, got %s for read of {%s} at version %" PRId64
+                    "\n",
+                    resultToStr(results2[i]), resultToStr(results1[i]),
+                    printable(reads[i].begin).c_str(), reads[i].readVersion);
+          } else {
+            fprintf(
+                stderr,
+                "Expected %s, got %s for read of [%s, %s) at version %" PRId64
+                "\n",
+                resultToStr(results2[i]), resultToStr(results1[i]),
+                printable(reads[i].begin).c_str(),
+                printable(reads[i].end).c_str(), reads[i].readVersion);
           }
+          ok = false;
+          return true;
         }
-        return true;
-      };
-
-      if (!compareResults(results1, results2, numPointReads + numRangeReads)) {
-        ok = false;
-        return true;
       }
-
-#ifdef THREAD_TEST
-      thread2.join();
-      if (!compareResults(results3, results2, numPointReads + numRangeReads)) {
-        ok = false;
-        return true;
-      }
-#endif
     }
     return false;
   }

Jenkinsfile

@@ -36,29 +36,6 @@ pipeline {
                 sh 'pre-commit run --all-files --show-diff-on-failure'
             }
         }
-        stage('Clang') {
-            agent {
-                dockerfile {
-                    args '-v /home/jenkins/ccache:/ccache'
-                    reuseNode true
-                }
-            }
-            steps {
-                CleanBuildAndTest("")
-                recordIssues(tools: [clang()])
-            }
-        }
-        stage('SIMD fallback') {
-            agent {
-                dockerfile {
-                    args '-v /home/jenkins/ccache:/ccache'
-                    reuseNode true
-                }
-            }
-            steps {
-                CleanBuildAndTest("-DUSE_SIMD_FALLBACK=ON")
-            }
-        }
         stage('Release [gcc]') {
             agent {
                 dockerfile {
@@ -67,7 +44,7 @@ pipeline {
                 }
             }
             steps {
-                CleanBuildAndTest("-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_CXX_FLAGS=-DNVALGRIND")
+                CleanBuildAndTest("-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_BUILD_TYPE=Release")
                 recordIssues(tools: [gcc()])
                 sh '''
                 cd build
@@ -89,7 +66,7 @@ pipeline {
                 }
             }
             steps {
-                CleanBuildAndTest("-DCMAKE_TOOLCHAIN_FILE=../aarch64-toolchain.cmake -DCMAKE_CXX_FLAGS=-DNVALGRIND")
+                CleanBuildAndTest("-DCMAKE_TOOLCHAIN_FILE=../aarch64-toolchain.cmake")
                 sh '''
                 cd build
                 cpack -G DEB
@@ -108,7 +85,7 @@ pipeline {
             steps {
                 CleanBuildAndTest("-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_C_FLAGS=--coverage -DCMAKE_CXX_FLAGS=--coverage -DCMAKE_BUILD_TYPE=Debug")
                 sh '''
-                gcovr -f ConflictSet.cpp --cobertura > build/coverage.xml
+                gcovr --exclude '.*third_party.*' --cobertura > build/coverage.xml
                 '''
                 cobertura autoUpdateHealth: false, autoUpdateStability: false, coberturaReportFile: 'build/coverage.xml', conditionalCoverageTargets: '70, 0, 0', failUnhealthy: false, failUnstable: false, lineCoverageTargets: '80, 0, 0', maxNumberOfBuilds: 0, methodCoverageTargets: '80, 0, 0', onlyStable: false, sourceEncoding: 'ASCII', zoomCoverageChart: false
             }

README.md

@@ -2,56 +2,54 @@ A data structure for optimistic concurrency control on ranges of bitwise-lexicog
 
 Intended to replace FoundationDB's skip list.
 
-Hardware for all benchmarks is a mac m1 2020.
-
 # FoundationDB's benchmark
 
 ## Skip list
 
 ```
-New conflict set: 1.957 sec
-                  0.639 Mtransactions/sec
-                  2.555 Mkeys/sec
-Detect only:      1.845 sec
-                  0.678 Mtransactions/sec
-                  2.710 Mkeys/sec
-Skiplist only:    1.263 sec
-                  0.990 Mtransactions/sec
-                  3.960 Mkeys/sec
+New conflict set: 2.404 sec
+                  0.520 Mtransactions/sec
+                  2.080 Mkeys/sec
+Detect only:      2.266 sec
+                  0.552 Mtransactions/sec
+                  2.207 Mkeys/sec
+Skiplist only:    1.594 sec
+                  0.784 Mtransactions/sec
+                  3.137 Mkeys/sec
 Performance counters:
-               Build: 0.0546
-                 Add: 0.0563
-              Detect: 1.84
-              D.Sort: 0.412
-           D.Combine: 0.0141
-         D.CheckRead: 0.671
-   D.CheckIntraBatch: 0.0068
-        D.MergeWrite: 0.592
-      D.RemoveBefore: 0.146
+               Build: 0.071
+                 Add: 0.0641
+              Detect: 2.27
+              D.Sort: 0.44
+           D.Combine: 0.018
+         D.CheckRead: 0.855
+   D.CheckIntraBatch: 0.00903
+        D.MergeWrite: 0.739
+      D.RemoveBefore: 0.201
 ```
 
 ## Radix tree (this implementation)
 
 ```
-New conflict set: 1.366 sec
-                  0.915 Mtransactions/sec
-                  3.660 Mkeys/sec
-Detect only:      1.248 sec
-                  1.002 Mtransactions/sec
-                  4.007 Mkeys/sec
-Skiplist only:    0.573 sec
-                  2.182 Mtransactions/sec
-                  8.730 Mkeys/sec
+New conflict set: 1.743 sec
+                  0.717 Mtransactions/sec
+                  2.869 Mkeys/sec
+Detect only:      1.611 sec
+                  0.776 Mtransactions/sec
+                  3.103 Mkeys/sec
+Skiplist only:    0.919 sec
+                  1.360 Mtransactions/sec
+                  5.440 Mkeys/sec
 Performance counters:
-               Build: 0.0594
-                 Add: 0.0572
-              Detect: 1.25
-              D.Sort: 0.418
-           D.Combine: 0.0149
-         D.CheckRead: 0.232
-   D.CheckIntraBatch: 0.0067
-        D.MergeWrite: 0.341
-      D.RemoveBefore: 0.232
+               Build: 0.0657
+                 Add: 0.0628
+              Detect: 1.61
+              D.Sort: 0.442
+           D.Combine: 0.0178
+         D.CheckRead: 0.395
+   D.CheckIntraBatch: 0.00776
+        D.MergeWrite: 0.524
+      D.RemoveBefore: 0.221
 ```
 
 # Our benchmark
@@ -60,25 +58,25 @@ Performance counters:
 
 |               ns/op |                op/s |    err% |     total | benchmark
 |--------------------:|--------------------:|--------:|----------:|:----------
-|              246.99 |        4,048,700.59 |    0.2% |      0.01 | `point reads`
-|              260.16 |        3,843,784.65 |    0.1% |      0.01 | `prefix reads`
-|              493.35 |        2,026,953.19 |    0.1% |      0.01 | `range reads`
-|              462.05 |        2,164,289.23 |    0.6% |      0.01 | `point writes`
-|              448.19 |        2,231,205.25 |    0.9% |      0.01 | `prefix writes`
-|              255.83 |        3,908,845.72 |    1.5% |      0.02 | `range writes`
-|              582.63 |        1,716,349.02 |    1.3% |      0.01 | `monotonic increasing point writes`
+|              270.07 |        3,702,706.03 |    0.4% |      0.01 | `point reads`
+|              285.76 |        3,499,437.03 |    1.5% |      0.01 | `prefix reads`
+|              532.54 |        1,877,794.90 |    0.7% |      0.01 | `range reads`
+|              528.50 |        1,892,132.94 |    0.7% |      0.01 | `point writes`
+|              516.53 |        1,935,978.22 |    0.9% |      0.01 | `prefix writes`
+|              303.34 |        3,296,630.84 |    3.6% |      0.05 | `range writes`
+|              502.88 |        1,988,553.24 |    2.0% |      0.01 | `monotonic increasing point writes`
 
 ## Radix tree (this implementation)
 
 |               ns/op |                op/s |    err% |     total | benchmark
 |--------------------:|--------------------:|--------:|----------:|:----------
-|               19.42 |       51,483,206.67 |    0.3% |      0.01 | `point reads`
-|               58.43 |       17,115,612.57 |    0.1% |      0.01 | `prefix reads`
-|              216.09 |        4,627,766.60 |    0.2% |      0.01 | `range reads`
-|               28.35 |       35,267,567.72 |    0.2% |      0.01 | `point writes`
-|               43.43 |       23,026,226.17 |    0.2% |      0.01 | `prefix writes`
-|               50.00 |       20,000,000.00 |    0.0% |      0.01 | `range writes`
-|               92.38 |       10,824,863.69 |    4.1% |      0.01 | `monotonic increasing point writes`
+|               14.52 |       68,850,842.99 |    1.2% |      0.01 | `point reads`
+|               60.89 |       16,422,538.22 |    1.5% |      0.01 | `prefix reads`
+|              226.89 |        4,407,362.98 |    0.5% |      0.01 | `range reads`
+|               22.99 |       43,498,198.49 |    0.2% |      0.01 | `point writes`
+|               50.51 |       19,799,864.54 |    1.0% |      0.01 | `prefix writes`
+|               82.50 |       12,121,212.12 |    2.6% |      0.03 | `range writes`
+|              119.94 |        8,337,354.54 |    2.1% |      0.01 | `monotonic increasing point writes`
 
 # "Real data" test
 
@@ -87,13 +85,13 @@ Point queries only, best of three runs. Gc ratio is the ratio of time spent doin
 ## skip list
 
 ```
-Check: 11.3385 seconds, 329.718 MB/s, Add: 5.35612 seconds, 131.072 MB/s, Gc ratio: 45.7173%
+Check: 12.7863 seconds, 292.384 MB/s, Add: 19.8276 seconds, 35.4071 MB/s, Gc ratio: 23.5314%
 ```
 
 ## radix tree
 
 ```
-Check: 2.48583 seconds, 1503.93 MB/s, Add: 2.12768 seconds, 329.954 MB/s, Gc ratio: 41.7943%
+Check: 3.60187 seconds, 1037.94 MB/s, Add: 3.03958 seconds, 230.966 MB/s, Gc ratio: 52.3876%
 ```
 
 ## hash table
@@ -101,5 +99,5 @@ Check: 2.48583 seconds, 1503.93 MB/s, Add: 2.12768 seconds, 329.954 MB/s, Gc rat
 (The hash table implementation doesn't work on range queries, and its purpose is to provide an idea of how fast point queries can be)
 
 ```
-Check: 1.83386 seconds, 2038.6 MB/s, Add: 0.601411 seconds, 1167.32 MB/s, Gc ratio: 48.9776%
-```
+Check: 2.15925 seconds, 1731.4 MB/s, Add: 1.08519 seconds, 646.926 MB/s, Gc ratio: 52.1526%
+```

RealDataBench.cpp

@@ -8,8 +8,6 @@
 #include <unistd.h>
 #include <vector>
 
-using namespace weaselab;
-
 double now() {
   return std::chrono::duration_cast<std::chrono::nanoseconds>(
              std::chrono::steady_clock::now().time_since_epoch())
@@ -30,7 +28,7 @@ constexpr inline size_t rightAlign(size_t offset, size_t alignment) {
 
 int main(int argc, const char **argv) {
   // Use with this dataset https://snap.stanford.edu/data/memetracker9.html
-  // Preprocess the files with `sed -i'' '/^Q/d'`
+  // Preprocess the files with `sed -i '' '/^Q/d'`
 
   double checkTime = 0;
   double addTime = 0;
@@ -42,8 +40,6 @@ int main(int argc, const char **argv) {
   int64_t version = 0;
   double timer = 0;
 
-  int64_t peakMemory = 0;
-
   for (int i = 1; i < argc; ++i) {
     int fd = open(argv[i], O_RDONLY);
     struct stat st;
@@ -113,10 +109,6 @@ int main(int argc, const char **argv) {
         write = {};
         reads.clear();
 
-        if (cs.getBytes() > peakMemory) {
-          peakMemory = cs.getBytes();
-        }
-
         timer = now();
         cs.setOldestVersion(version - 10000);
         gcTime += now() - timer;
@@ -126,9 +118,8 @@ int main(int argc, const char **argv) {
     close(fd);
   }
 
-  printf("Check: %g seconds, %g MB/s, Add: %g seconds, %g MB/s, Gc ratio: "
-         "%g%%, Peak idle memory: %g\n",
-         checkTime, checkBytes / checkTime * 1e-6, addTime,
-         addBytes / addTime * 1e-6, gcTime / (gcTime + addTime) * 1e2,
-         double(peakMemory));
+  printf(
+      "Check: %g seconds, %g MB/s, Add: %g seconds, %g MB/s, Gc ratio: %g%%\n",
+      checkTime, checkBytes / checkTime * 1e-6, addTime,
+      addBytes / addTime * 1e-6, gcTime / (gcTime + addTime) * 1e2);
 }

ScriptTest.cpp

@@ -1,155 +0,0 @@
-#include "Internal.h"
-#include <ConflictSet.h>
-
-#include <chrono>
-#include <cstring>
-#include <fcntl.h>
-#include <sys/mman.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <vector>
-
-inline size_t getPageSize() {
-  static size_t kPageSize = sysconf(_SC_PAGESIZE);
-  return kPageSize;
-}
-
-/// Helper for rounding up to page size (or some other alignment)
-constexpr inline size_t rightAlign(size_t offset, size_t alignment) {
-  return offset % alignment == 0 ? offset
-                                 : ((offset / alignment) + 1) * alignment;
-}
-
-using StringView = std::basic_string_view<uint8_t>;
-
-inline StringView operator"" _v(const char *str, size_t size) {
-  return {reinterpret_cast<const uint8_t *>(str), size};
-}
-
-int main(int argc, const char **argv) {
-
-  ConflictSet cs{0};
-  ReferenceImpl ref{0};
-
-  for (int i = 1; i < argc; ++i) {
-    int fd = open(argv[i], O_RDONLY);
-    struct stat st;
-    if (fstat(fd, &st) == -1) {
-      int err = errno;
-      fprintf(stderr, "stat error %s - %s\n", argv[i], strerror(err));
-      fflush(stderr);
-      abort();
-    }
-
-    int64_t size = rightAlign(st.st_size, getPageSize());
-    const uint8_t *begin =
-        (uint8_t *)mmap(0, size, PROT_READ, MAP_PRIVATE, fd, 0);
-    madvise((void *)begin, size, MADV_SEQUENTIAL);
-    auto *const mapOriginal = begin;
-    const auto sizeOriginal = size;
-
-    StringView b;
-    StringView e;
-    int64_t v = 0;
-    int64_t lastWriteVersion = 0;
-    int64_t lastOldestVersion = 0;
-    std::vector<ConflictSet::WriteRange> writeRanges;
-    std::vector<ConflictSet::ReadRange> readRanges;
-    std::vector<ConflictSet::Result> results;
-
-    for (uint8_t *end = (uint8_t *)memchr(begin, '\n', size); end != nullptr;) {
-      StringView line{begin, static_cast<size_t>(end - begin)};
-      size -= end - begin + 1;
-      begin = end + 1;
-      end = (uint8_t *)memchr(begin, '\n', size);
-
-      if (line.starts_with("begin"_v)) {
-        b = line.substr("begin "_v.size(), line.size());
-        printf("b <- %.*s\n", int(b.size()), b.data());
-      } else if (line.starts_with("end"_v)) {
-        e = line.substr("end "_v.size(), line.size());
-        printf("e <- %.*s\n", int(e.size()), e.data());
-      } else if (line.starts_with("version"_v)) {
-        line = line.substr("version "_v.size(), line.size());
-        v = 0;
-        for (auto c : line) {
-          v = v * 10 + int(c) - int('0');
-        }
-        printf("v <- %" PRId64 "\n", v);
-      } else if (line.starts_with("pointread"_v)) {
-        printf("pointread\n");
-        ConflictSet::ReadRange r;
-        r.begin.p = b.data();
-        r.begin.len = b.size();
-        r.end.len = 0;
-        r.readVersion = v;
-        readRanges.push_back(r);
-      } else if (line.starts_with("pointwrite"_v)) {
-        printf("pointwrite\n");
-        assert(writeRanges.empty() ||
-               (writeRanges.back().end.len == 0 ? writeRanges.back().begin
-                                                : writeRanges.back().end) < b);
-        ConflictSet::WriteRange w;
-        w.begin.p = b.data();
-        w.begin.len = b.size();
-        w.end.len = 0;
-        writeRanges.push_back(w);
-      } else if (line.starts_with("rangeread"_v)) {
-        printf("rangeread\n");
-        ConflictSet::ReadRange r;
-        r.begin.p = b.data();
-        r.begin.len = b.size();
-        r.end.p = e.data();
-        r.end.len = e.size();
-        r.readVersion = v;
-        readRanges.push_back(r);
-      } else if (line.starts_with("rangewrite"_v)) {
-        printf("rangewrite\n");
-        assert(b < e);
-        assert(writeRanges.empty() ||
-               (writeRanges.back().end.len == 0 ? writeRanges.back().begin
-                                                : writeRanges.back().end) < b);
-        ConflictSet::WriteRange w;
-        w.begin.p = b.data();
-        w.begin.len = b.size();
-        w.end.p = e.data();
-        w.end.len = e.size();
-        writeRanges.push_back(w);
-      } else if (line.starts_with("check"_v)) {
-        printf("check\n");
-        Arena arena;
-        auto *expected = new (arena) ConflictSet::Result[readRanges.size()];
-        auto *actual = new (arena) ConflictSet::Result[readRanges.size()];
-        ref.check(readRanges.data(), expected, readRanges.size());
-        cs.check(readRanges.data(), actual, readRanges.size());
-        for (int i = 0; i < int(readRanges.size()); ++i) {
-          if (expected[i] != actual[i]) {
-            fprintf(stderr, "Expected %s, got %s at index %d\n",
-                    resultToStr(expected[i]), resultToStr(actual[i]), i);
-            return 1;
-          }
-        }
-        readRanges = {};
-      } else if (line.starts_with("addwrites"_v)) {
-        printf("addwrites\n");
-        assert(v > lastWriteVersion);
-        lastWriteVersion = v;
-        cs.addWrites(writeRanges.data(), writeRanges.size(), v);
-        ref.addWrites(writeRanges.data(), writeRanges.size(), v);
-        writeRanges = {};
-      } else if (line.starts_with("setoldest"_v)) {
-        printf("setoldest\n");
-        assert(v > lastOldestVersion);
-        lastOldestVersion = v;
-        cs.setOldestVersion(v);
-        ref.setOldestVersion(v);
-      } else if (line.empty() || line.starts_with(";"_v)) {
-        // skip
-      } else {
-        printf("Unrecognized line: %.*s\n", int(line.size()), line.data());
-      }
-    }
-    munmap((void *)mapOriginal, sizeOriginal);
-    close(fd);
-  }
-}

SkipList.cpp

@@ -149,7 +149,7 @@ private:
       setMaxVersion(level, v);
     }
 
-    void destroy() { safe_free(this, getNodeSize()); }
+    void destroy() { free(this); }
 
   private:
     int getNodeSize() const {
@@ -607,7 +607,7 @@ 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]);
-        keyUpdates += 3;
+        keyUpdates += 2;
       }
       skipList.find(values, fingers, temp, ss);
       skipList.addConflictRanges(fingers, ss / 2, writeVersion);
@@ -621,16 +621,14 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     int temp;
     std::span<const uint8_t> key = removalKey;
     skipList.find(&key, &finger, &temp, 1);
-    skipList.removeBefore(oldestVersion, finger, std::exchange(keyUpdates, 10));
+    skipList.removeBefore(oldestVersion, finger, std::exchange(keyUpdates, 0));
     removalArena = Arena();
     removalKey = copyToArena(
         removalArena, {finger.getValue().data(), finger.getValue().size()});
   }
 
-  int64_t totalBytes = 0;
-
 private:
-  int64_t keyUpdates = 10;
+  int64_t keyUpdates = 0;
   Arena removalArena;
   std::span<const uint8_t> removalKey;
   int64_t oldestVersion;
@@ -639,44 +637,25 @@ private:
 
 void ConflictSet::check(const ReadRange *reads, Result *results,
                         int count) const {
-  impl->check(reads, results, count);
+  return impl->check(reads, results, count);
 }
 
 void ConflictSet::addWrites(const WriteRange *writes, int count,
                             int64_t writeVersion) {
-  mallocBytesDelta = 0;
-  impl->addWrites(writes, count, writeVersion);
-  impl->totalBytes += mallocBytesDelta;
-#if SHOW_MEMORY
-  if (impl->totalBytes != mallocBytes) {
-    abort();
-  }
-#endif
+  return impl->addWrites(writes, count, writeVersion);
 }
 
 void ConflictSet::setOldestVersion(int64_t oldestVersion) {
-  mallocBytesDelta = 0;
-  impl->setOldestVersion(oldestVersion);
-  impl->totalBytes += mallocBytesDelta;
-#if SHOW_MEMORY
-  if (impl->totalBytes != mallocBytes) {
-    abort();
-  }
-#endif
+  return impl->setOldestVersion(oldestVersion);
 }
 
-int64_t ConflictSet::getBytes() const { return impl->totalBytes; }
-
 ConflictSet::ConflictSet(int64_t oldestVersion)
-    : impl((mallocBytesDelta = 0,
-            new (safe_malloc(sizeof(Impl))) Impl{oldestVersion})) {
-  impl->totalBytes += mallocBytesDelta;
-}
+    : impl(new (safe_malloc(sizeof(Impl))) Impl{oldestVersion}) {}
 
 ConflictSet::~ConflictSet() {
   if (impl) {
     impl->~Impl();
-    safe_free(impl, sizeof(Impl));
+    free(impl);
   }
 }
 
@@ -716,20 +695,6 @@ ConflictSet_create(int64_t oldestVersion) {
 __attribute__((__visibility__("default"))) void ConflictSet_destroy(void *cs) {
   using Impl = ConflictSet::Impl;
   ((Impl *)cs)->~Impl();
-  safe_free(cs, sizeof(Impl));
-}
-__attribute__((__visibility__("default"))) int64_t
-ConflictSet_getBytes(void *cs) {
-  using Impl = ConflictSet::Impl;
-  return ((Impl *)cs)->totalBytes;
+  free(cs);
 }
 }
-
-#if SHOW_MEMORY
-struct __attribute__((visibility("default"))) PeakPrinter {
-  ~PeakPrinter() {
-    printf("malloc bytes: %g\n", double(mallocBytes));
-    printf("Peak malloc bytes: %g\n", double(peakMallocBytes));
-  }
-} peakPrinter;
-#endif

TestDriver.cpp

@@ -5,6 +5,7 @@
 
 int main(int argc, char **argv) {
   for (int i = 1; i < argc; ++i) {
+    printf("Running: %s\n", argv[i]);
     std::ifstream t(argv[i], std::ios::binary);
     std::stringstream buffer;
     buffer << t.rdbuf();

conflict_set_c_api_test.c

@@ -7,7 +7,6 @@ int main(void) {
   ConflictSet_WriteRange w;
   ConflictSet_Result result;
   ConflictSet_ReadRange r;
-  int64_t bytes;
   w.begin.p = (const uint8_t *)"0000";
   w.begin.len = 4;
   w.end.len = 0;
@@ -18,8 +17,6 @@ int main(void) {
   r.readVersion = 0;
   ConflictSet_check(cs, &r, &result, 1);
   assert(result == ConflictSet_Conflict);
-  bytes = ConflictSet_getBytes(cs);
-  assert(bytes > 0);
   ConflictSet_destroy(cs);
   return 0;
 }

conflict_set_cxx_api_test.cpp

@@ -2,8 +2,6 @@
 
 #include <cassert>
 
-using namespace weaselab;
-
 int main(void) {
   ConflictSet cs(0);
   ConflictSet::WriteRange w;
@@ -19,6 +17,4 @@ int main(void) {
   r.readVersion = 0;
   cs.check(&r, &result, 1);
   assert(result == ConflictSet::Conflict);
-  int64_t bytes = cs.getBytes();
-  assert(bytes > 0);
 }