Deallocation code path optimisations

- Removal of the lock - Minor fixes around the allocation code paths
DataDog · Aug 30, 2023 · c182378 · c182378
1 parent fc9ae2d
commit c182378
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 21 deletions.
diff --git a/include/lib/address_bitset.hpp b/include/lib/address_bitset.hpp
@@ -13,7 +13,7 @@ class AddressBitset {
   // returns true if the element was removed
   bool unset(uintptr_t addr);
   void clear() {
-    for (auto& element : _address_bitset) {
+    for (auto &element : _address_bitset) {
       element.store(0, std::memory_order_relaxed);
     }
     _nb_elements.store(0);

diff --git a/src/lib/allocation_tracker.cc b/src/lib/allocation_tracker.cc
@@ -271,11 +271,8 @@ void AllocationTracker::track_deallocation(uintptr_t addr,
     return;
   }
 
-  { // Grab the lock to check if this allocation was stored in the set
-    std::lock_guard lock{_state.mutex};
-    if (!_state.track_deallocations || !_dealloc_bitset.unset(addr)) {
-      return;
-    }
+  if (!_state.track_deallocations || !_dealloc_bitset.unset(addr)) {
+    return;
   }
 
   bool success = IsDDResOK(push_dealloc_sample(addr, tl_state));

diff --git a/test/address_bitset-ut.cc b/test/address_bitset-ut.cc
@@ -17,8 +17,8 @@ TEST(address_bitset, many_addresses) {
   AddressBitset address_bitset;
   std::random_device rd;
   std::mt19937 gen(rd());
-  std::uniform_int_distribution<uintptr_t> dis(0,
-                                               std::numeric_limits<uintptr_t>::max());
+  std::uniform_int_distribution<uintptr_t> dis(
+      0, std::numeric_limits<uintptr_t>::max());
 
   std::vector<uintptr_t> addresses;
   unsigned nb_elements = 100000;
@@ -34,4 +34,4 @@ TEST(address_bitset, many_addresses) {
   }
   EXPECT_EQ(0, address_bitset.nb_elements());
 }
-}
+} // namespace ddprof
diff --git a/test/allocation_tracker-bench.cc b/test/allocation_tracker-bench.cc
@@ -17,7 +17,9 @@
 // Sampling rate: default rate is 524288
 static constexpr uint64_t k_rate = 200000;
 
-#define READER_THREAD
+// The reader thread is interesting, though it starts dominating the CPU
+// the benchmark focuses on the capture of allocation events.
+// #define READER_THREAD
 std::atomic<bool> reader_continue{true};
 std::atomic<bool> error_in_reader{false};
 
@@ -27,17 +29,14 @@ void read_buffer(ddprof::RingBufferHolder &holder) {
   error_in_reader = false;
   while (reader_continue) {
     ddprof::MPSCRingBufferReader reader(holder.get_ring_buffer());
-//    fprintf(stderr, "size = %lu! \n", reader.available_size());
+    //    fprintf(stderr, "size = %lu! \n", reader.available_size());
     auto buf = reader.read_sample();
     if (!buf.empty()) {
       ++nb_samples;
-//      fprintf(stderr, "Yep, got sample ! \n");
     }
     std::chrono::microseconds(10000);
   }
-#ifdef  DEBUG
   fprintf(stderr, "Reader thread exit, nb_samples=%d\n", nb_samples);
-#endif
   if (nb_samples == 0) {
     error_in_reader = true;
   }
@@ -70,7 +69,6 @@ void perform_memory_operations(bool track_allocations,
       ddprof::AllocationTracker::kTrackDeallocations;
 #endif
 
-
   int nb_threads = 4;
   std::vector<std::thread> threads;
   int num_allocations = 1000;
@@ -100,7 +98,7 @@ void perform_memory_operations(bool track_allocations,
       threads.emplace_back([&, i] {
         ddprof::AllocationTracker::init_tl_state();
         std::uniform_int_distribution<uintptr_t> dis(i * page_size,
-                                                (i + 1) * page_size - 1);
+                                                     (i + 1) * page_size - 1);
 
         for (int j = 0; j < num_allocations; ++j) {
           uintptr_t addr = dis(gen);
@@ -116,7 +114,6 @@ void perform_memory_operations(bool track_allocations,
 
     threads.clear();
     for (int i = 0; i < nb_threads; ++i) {
-      ddprof::AllocationTracker::init_tl_state();
       threads.emplace_back([&, i] {
         ddprof::AllocationTracker::init_tl_state();
         for (auto addr : thread_addresses[i]) {
@@ -130,8 +127,6 @@ void perform_memory_operations(bool track_allocations,
     }
   }
 
-
-
 #ifdef READER_THREAD
   reader_continue = false;
   reader_thread.join();

diff --git a/test/allocation_tracker-ut.cc b/test/allocation_tracker-ut.cc
@@ -165,8 +165,7 @@ TEST(allocation_tracker, max_tracked_allocs) {
         ASSERT_EQ(sample->pid, getpid());
         ASSERT_EQ(sample->tid, ddprof::gettid());
         ASSERT_EQ(sample->addr, 0x1000 + i);
-      }
-      else {
+      } else {
         if (hdr->type == PERF_CUSTOM_EVENT_CLEAR_LIVE_ALLOCATION) {
           printf("Clear was found at iteration %d\n", i);
           clear_found = true;