fix(nativemem): count NM_CALLTRACE residency, not reserved chunk capacity

ddprof-lib/src/main/cpp/linearAllocator.cpp

@@ -46,6 +46,14 @@ LinearAllocator::~LinearAllocator() {
 }
 
 void LinearAllocator::clear() {
+  // Both pointers are NULL only after detachChunks() could not allocate a
+  // replacement chunk, which leaves the allocator deliberately unusable rather
+  // than risking a double free. Bail out here so the whole function is
+  // null-safe: it dereferences _reserve immediately below and _tail at the end,
+  // so guarding either one alone would leave the other exposed.
+  if (_tail == NULL || _reserve == NULL) {
+    return;
+  }
   // OS::safeAlloc/safeFree use raw syscalls not intercepted by TSan, so TSan
   // never clears shadow memory on munmap.  Add explicit acquire/release around
   // every plain prev-field read so the happens-before chain from freeChunk's
@@ -102,6 +110,10 @@ void LinearAllocator::clear() {
     _reserve = current;
     _tail = current;
   }
+  // _tail is kept (not freed) but its contents are discarded here, so its
+  // consumed bytes must be un-recorded explicitly -- freeChunk() won't run
+  // for this one.
+  NativeMem::record(NM_CALLTRACE, -(long long)(_tail->offs - sizeof(Chunk)));
   _tail->offs = sizeof(Chunk);
 
   // DON'T UNPOISON HERE - let alloc() do it on-demand!
@@ -162,15 +174,17 @@ void LinearAllocator::freeChunks(ChunkList& chunks) {
     __tsan_acquire(current);
     #endif
     Chunk* prev = current->prev;
+    // Capture before safeFree unmaps the chunk -- reading current->offs after
+    // that would touch freed memory. This is exactly the byte count alloc()
+    // recorded into NM_CALLTRACE for this chunk, so the decrement is exact.
+    long long used = (long long)(current->offs - sizeof(Chunk));
     #ifdef TSAN_ENABLED
     __tsan_release(current);
     #endif
     OS::safeFree(current, chunks.chunk_size);
     Counters::decrement(LINEAR_ALLOCATOR_BYTES, chunks.chunk_size);
     Counters::decrement(LINEAR_ALLOCATOR_CHUNKS);
-    // The LinearAllocator's only user is call-trace storage, so all of its
-    // chunk memory is attributed to the CALLTRACE category.
-    NativeMem::record(NM_CALLTRACE, -(long long)chunks.chunk_size);
+    NativeMem::record(NM_CALLTRACE, -used);
     current = prev;
   }
 
@@ -195,6 +209,11 @@ void *LinearAllocator::alloc(size_t size) {
       if (__sync_bool_compare_and_swap(&chunk->offs, offs, offs + size)) {
         void* allocated_ptr = (char *)chunk + offs;
 
+        // The LinearAllocator's only user is call-trace storage, so all of
+        // its bump-allocated bytes are attributed to the CALLTRACE category.
+        // A relaxed atomic add, safe to call from the sampling signal handler.
+        NativeMem::record(NM_CALLTRACE, (long long)size);
+
         // ASAN UNPOISONING: Unpoison ONLY the allocated region on-demand
         // This allows ASan to detect use-after-free of memory that was cleared
         // but not yet reallocated
@@ -264,12 +283,30 @@ Chunk *LinearAllocator::allocateChunk(Chunk *current) {
 
     Counters::increment(LINEAR_ALLOCATOR_BYTES, _chunk_size);
     Counters::increment(LINEAR_ALLOCATOR_CHUNKS);
-    NativeMem::record(NM_CALLTRACE, (long long)_chunk_size);
+    // NM_CALLTRACE is NOT recorded here. Recording the full chunk size at
+    // reservation time would count virtual capacity, not residency: the
+    // chunk is mmap'd whole but filled incrementally by alloc()'s bump
+    // pointer, and reserveChunk() eagerly reserves the next chunk at 50%
+    // fill of the current one, so there is always at least one chunk that's
+    // fully counted but mostly or entirely untouched. alloc() instead
+    // records exactly the bytes actually handed out, which -- because the
+    // bump pointer advances linearly and every returned pointer is written
+    // into immediately by the caller -- tracks touched (resident) bytes
+    // directly, with no separate residency measurement needed.
   }
   return chunk;
 }
 
 void LinearAllocator::freeChunk(Chunk *current) {
+  // allocateChunk() returns NULL when the mmap fails, and detachChunks() stores
+  // that NULL into _tail, so the destructor's freeChunk(_tail) can be reached
+  // with nothing to free. Bail out rather than dereference it below.
+  if (current == NULL) {
+    return;
+  }
+  // Capture before safeFree unmaps the chunk -- see freeChunks() for why this
+  // exactly reverses what alloc() recorded for this chunk.
+  long long used = (long long)(current->offs - sizeof(Chunk));
   // Release TSan ownership before munmap so the sanitizer knows this thread is
   // done with the memory.  The mmap(MAP_FIXED) re-map in allocateChunk() resets
   // the shadow for whichever thread later reuses this VA (after OS VA reuse), so
@@ -280,7 +317,7 @@ void LinearAllocator::freeChunk(Chunk *current) {
   OS::safeFree(current, _chunk_size);
   Counters::decrement(LINEAR_ALLOCATOR_BYTES, _chunk_size);
   Counters::decrement(LINEAR_ALLOCATOR_CHUNKS);
-  NativeMem::record(NM_CALLTRACE, -(long long)_chunk_size);
+  NativeMem::record(NM_CALLTRACE, -used);
 }
 
 void LinearAllocator::reserveChunk(Chunk *current) {

ddprof-lib/src/main/cpp/nativeMem.h

@@ -13,10 +13,17 @@
 // per-category live byte gauges partition the total: sum(category) == total,
 // with no double counting.
 //
-// The "reserved vs used vs wasted" breakdowns exposed by the existing counters
-// (CALLTRACE_STORAGE_BYTES is the used slice of the CALLTRACE arena;
-// DICTIONARY_ARENA_WASTE_BYTES is the wasted slice of the DICTIONARY arena) are
-// a separate, nested dimension. They are intentionally NOT summed in here.
+// These gauges track memory that is actually touched (and therefore resident),
+// not address space that has merely been reserved. NM_CALLTRACE in particular
+// counts the bytes LinearAllocator::alloc() has bump-allocated, NOT the
+// capacity of the 8 MiB chunks backing them -- see linearAllocator.cpp.
+//
+// Consequently NM_CALLTRACE is no longer an "arena reserved" figure with
+// CALLTRACE_STORAGE_BYTES as its used slice: both now count touched bytes, and
+// their small difference is the call-trace hash tables (also bump-allocated),
+// not chunk slack. Do not compute arena waste as the difference between them.
+// DICTIONARY_ARENA_WASTE_BYTES remains a genuine nested waste figure for the
+// DICTIONARY arena. None of these nested counters are summed in here.
 #define DD_NATIVE_MEM_CATEGORY_TABLE(X)                                        \
   X(CALLTRACE, "calltrace")                                                    \
   X(DICTIONARY, "dictionary")                                                  \

ddprof-lib/src/test/cpp/linearAllocator_nativemem_ut.cpp

@@ -0,0 +1,101 @@
+#include "gtest/gtest.h"
+#include "linearAllocator.h"
+#include "nativeMem.h"
+
+// NM_CALLTRACE must track bytes actually handed out by alloc() -- i.e. touched,
+// resident bytes -- not the virtual capacity of the mmap'd chunks backing them.
+// The distinction is large: chunks are 8 MiB (CALL_TRACE_CHUNK) and alloc()
+// eagerly reserves the next one at 50% fill, so a capacity-based counter
+// over-reports by up to two chunks' worth of untouched address space.
+
+static const size_t CHUNK_SIZE = 1024 * 1024;
+
+class LinearAllocatorNativeMemTest : public ::testing::Test {
+protected:
+  long long _baseline[NM_NUM_CATEGORIES];
+
+  // Static-duration objects elsewhere in the binary record into these same
+  // categories before main() runs; restore their baseline in TearDown so
+  // their destructors don't underflow a category this fixture zeroed.
+  // (Same rationale as NativeMemTest in nativeMem_ut.cpp.)
+  void SetUp() override {
+    for (int c = 0; c < NM_NUM_CATEGORIES; c++) {
+      _baseline[c] = NativeMem::live((NativeMemCategory)c);
+    }
+    NativeMem::reset();
+  }
+  void TearDown() override {
+    NativeMem::reset();
+    for (int c = 0; c < NM_NUM_CATEGORIES; c++) {
+      NativeMem::setLive((NativeMemCategory)c, _baseline[c]);
+    }
+  }
+};
+
+TEST_F(LinearAllocatorNativeMemTest, ReportsBumpedBytesNotChunkCapacity) {
+  const size_t ALLOC_SIZE = 128;
+  const int NUM_ALLOCS = 100;
+
+  LinearAllocator allocator(CHUNK_SIZE);
+  // Constructing the allocator mmaps a chunk but touches nothing in it.
+  EXPECT_EQ(0, NativeMem::live(NM_CALLTRACE))
+      << "a freshly reserved chunk is untouched and must not be counted";
+
+  for (int i = 0; i < NUM_ALLOCS; i++) {
+    ASSERT_NE(nullptr, allocator.alloc(ALLOC_SIZE));
+  }
+
+  EXPECT_EQ((long long)(ALLOC_SIZE * NUM_ALLOCS), NativeMem::live(NM_CALLTRACE))
+      << "live bytes must equal exactly the bytes alloc() handed out";
+  EXPECT_LT(NativeMem::live(NM_CALLTRACE), (long long)CHUNK_SIZE)
+      << "must not have jumped to whole-chunk granularity";
+}
+
+TEST_F(LinearAllocatorNativeMemTest, CrossingIntoASecondChunkStaysByteAccurate) {
+  // An allocation size that does not divide the chunk evenly, driven past the
+  // point where reserveChunk() pre-reserves the next chunk, so the reserved
+  // chunk's untouched capacity would show up in a capacity-based counter.
+  const size_t ALLOC_SIZE = 3000;
+  const int NUM_ALLOCS = (int)(CHUNK_SIZE / ALLOC_SIZE) + 50;
+
+  LinearAllocator allocator(CHUNK_SIZE);
+  long long handed_out = 0;
+  for (int i = 0; i < NUM_ALLOCS; i++) {
+    if (allocator.alloc(ALLOC_SIZE) != nullptr) {
+      handed_out += (long long)ALLOC_SIZE;
+    }
+  }
+
+  EXPECT_EQ(handed_out, NativeMem::live(NM_CALLTRACE))
+      << "byte-accurate across a chunk boundary, with a chunk pre-reserved";
+}
+
+TEST_F(LinearAllocatorNativeMemTest, ClearReturnsLiveToZero) {
+  LinearAllocator allocator(CHUNK_SIZE);
+  for (int i = 0; i < 50; i++) {
+    ASSERT_NE(nullptr, allocator.alloc(256));
+  }
+  ASSERT_GT(NativeMem::live(NM_CALLTRACE), 0);
+
+  allocator.clear();
+
+  EXPECT_EQ(0, NativeMem::live(NM_CALLTRACE))
+      << "clear() discards all chunk contents, including the retained _tail's";
+}
+
+TEST_F(LinearAllocatorNativeMemTest, FreeChunksReturnsLiveToZero) {
+  LinearAllocator allocator(CHUNK_SIZE);
+  for (int i = 0; i < 50; i++) {
+    ASSERT_NE(nullptr, allocator.alloc(256));
+  }
+  long long before_detach = NativeMem::live(NM_CALLTRACE);
+  ASSERT_GT(before_detach, 0);
+
+  ChunkList detached = allocator.detachChunks();
+  EXPECT_EQ(before_detach, NativeMem::live(NM_CALLTRACE))
+      << "detaching moves ownership but frees nothing yet";
+
+  LinearAllocator::freeChunks(detached);
+  EXPECT_EQ(0, NativeMem::live(NM_CALLTRACE))
+      << "freeing the detached chunks un-records exactly what alloc() recorded";
+}