remainder of comments

Author: byrnedj

cachelib/CMakeLists.txt

@@ -43,7 +43,7 @@ set(PACKAGE_BUGREPORT "https://github.com/facebook/TBD")
 set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 
 option(BUILD_TESTS "If enabled, compile the tests." ON)
-option(WITH_DTO "If enabled, build with DSA transparent offloading." OFF)
+option(BUILD_WITH_DTO "If enabled, build with DSA transparent offloading." OFF)
 
 
 set(BIN_INSTALL_DIR bin CACHE STRING

cachelib/allocator/BackgroundMover-inl.h

@@ -32,12 +32,10 @@ BackgroundMover<CacheT>::BackgroundMover(
 
 template <typename CacheT>
 void BackgroundMover<CacheT>::TraversalStats::recordTraversalTime(uint64_t nsTaken) {
-  lastTraversalTimeNs_.store(nsTaken, std::memory_order_relaxed);
-  minTraversalTimeNs_.store(std::min(minTraversalTimeNs_.load(), nsTaken),
-                            std::memory_order_relaxed);
-  maxTraversalTimeNs_.store(std::max(maxTraversalTimeNs_.load(), nsTaken),
-                            std::memory_order_relaxed);
-  totalTraversalTimeNs_.fetch_add(nsTaken, std::memory_order_relaxed);
+  lastTraversalTimeNs_ = nsTaken;
+  minTraversalTimeNs_ = std::min(minTraversalTimeNs_, nsTaken);
+  maxTraversalTimeNs_ = std::max(maxTraversalTimeNs_, nsTaken);
+  totalTraversalTimeNs_ += nsTaken;
 }
 
 template <typename CacheT>
@@ -100,8 +98,8 @@ void BackgroundMover<CacheT>::checkAndRun() {
     auto end = util::getCurrentTimeNs();
     if (moves > 0) {
       traversalStats_.recordTraversalTime(end > begin ? end - begin : 0);
-      numMovedItems.add(moves);
-      numTraversals.inc();
+      numMovedItems += moves;
+      numTraversals++;
     }
 
     //we didn't move any objects done with this run
@@ -114,12 +112,12 @@ void BackgroundMover<CacheT>::checkAndRun() {
 template <typename CacheT>
 BackgroundMoverStats BackgroundMover<CacheT>::getStats() const noexcept {
   BackgroundMoverStats stats;
-  stats.numMovedItems = numMovedItems.get();
-  stats.totalBytesMoved = totalBytesMoved.get();
-  stats.totalClasses = totalClasses.get();
+  stats.numMovedItems = numMovedItems;
+  stats.totalBytesMoved = totalBytesMoved;
+  stats.totalClasses = totalClasses;
   auto runCount = getRunCount();
   stats.runCount = runCount;
-  stats.numTraversals = numTraversals.get();
+  stats.numTraversals = numTraversals;
   stats.avgItemsMoved = (double) stats.numMovedItems / (double)runCount;
   stats.lastTraversalTimeNs = traversalStats_.getLastTraversalTimeNs();
   stats.avgTraversalTimeNs = traversalStats_.getAvgTraversalTimeNs(runCount);

cachelib/allocator/BackgroundMover.h

@@ -86,12 +86,12 @@ class BackgroundMover : public PeriodicWorker {
 
    private:
     // time it took us the last time to traverse the cache.
-    std::atomic<uint64_t> lastTraversalTimeNs_{0};
-    std::atomic<uint64_t> minTraversalTimeNs_{
+    uint64_t lastTraversalTimeNs_{0};
+    uint64_t minTraversalTimeNs_{
         std::numeric_limits<uint64_t>::max()};
-    std::atomic<uint64_t> maxTraversalTimeNs_{0};
-    std::atomic<uint64_t> totalTraversalTimeNs_{0};
-    std::atomic<uint64_t> numTraversals_{0};
+    uint64_t maxTraversalTimeNs_{0};
+    uint64_t totalTraversalTimeNs_{0};
+    uint64_t numTraversals_{0};
   };
 
   TraversalStats traversalStats_;
@@ -110,10 +110,10 @@ class BackgroundMover : public PeriodicWorker {
   void work() override final;
   void checkAndRun();
 
-  AtomicCounter numMovedItems{0};
-  AtomicCounter numTraversals{0};
-  AtomicCounter totalClasses{0};
-  AtomicCounter totalBytesMoved{0};
+  uint64_t numMovedItems{0};
+  uint64_t numTraversals{0};
+  uint64_t totalClasses{0};
+  uint64_t totalBytesMoved{0};
 
   std::vector<MemoryDescriptorType> assignedMemory_;
   folly::DistributedMutex mutex;

cachelib/cachebench/CMakeLists.txt

@@ -49,7 +49,7 @@ endif()
 add_executable (cachebench main.cpp)
 target_link_libraries(cachebench cachelib_cachebench)
 
-if (WITH_DTO)
+if (BUILD_WITH_DTO)
     target_link_libraries(cachebench accel-config dto)
 endif ()