fix(nativemem): refresh NM_NATIVE_SYMBOLS where the tables change

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

@@ -71,6 +71,9 @@ CodeCache::CodeCache(const char *name, short lib_index,
   _count = 0;
   _blobs = new CodeBlob[_capacity];
 
+  _memory_usage = (long long)_capacity * sizeof(CodeBlob) +
+                  (long long)NativeFunc::allocSize(_name);
+
   _published.store(false, std::memory_order_relaxed);
 }
 
@@ -125,6 +128,11 @@ void CodeCache::copyFrom(const CodeCache& other) {
     }
   }
 
+  // Identical content to `other`, so its running total transfers as-is --
+  // no need to recompute (and `other` may itself already be published, whose
+  // _blobs this copy must not depend on after construction).
+  _memory_usage = other._memory_usage;
+
   // A copy is a fresh, not-yet-registered cache.
   _published.store(false, std::memory_order_relaxed);
 }
@@ -159,35 +167,19 @@ CodeCache::~CodeCache() {
 }
 
 long long CodeCache::memoryUsage() const {
-  // The blob array: _capacity entries of CodeBlob.
-  long long total = (long long)_capacity * sizeof(CodeBlob);
-
-  // This cache's own name, plus each symbol's name string. Each is a
-  // variable-length allocation whose size depends on the name length
-  // (see NativeFunc::allocSize). The lock-free read here is safe only for caches
-  // registered in a CodeCacheArray (Libraries::native_libs): their _blobs array
-  // and name pointers are fixed once published (add()/expand()/setDwarfTable()
-  // run only pre-publish — asserted via _published), which is the same read the
-  // symbolication fast path relies on. It must NOT be called on a continuously
-  // mutated, unpublished cache such as JitCodeCache::_runtime_stubs without
-  // holding JitCodeCache::_stubs_lock (shared), since a concurrent add()/expand()
-  // would free _blobs underneath the reader.
-  total += (long long)NativeFunc::allocSize(_name);
-  for (int i = 0; i < _count; i++) {
-    total += (long long)NativeFunc::allocSize(_blobs[i]._name);
-  }
-
-  // The DWARF unwind table, when present (length only — no pointer deref).
-  total += (long long)_dwarf_table_length * sizeof(FrameDesc);
-
-  // The build-id string is intentionally NOT counted here: the background
-  // library refresher (Libraries::updateBuildIds) frees and replaces _build_id
-  // on already-published caches under _build_id_lock, which dump does not hold,
-  // so dereferencing it here would race. It is negligible (~tens of bytes per
-  // library) next to the symbol tables, so excluding it costs no meaningful
-  // accuracy while keeping this read lock-free.
-
-  return total;
+  // O(1): _memory_usage is updated incrementally by the constructor, add(),
+  // expand(), and setDwarfTable() -- the same pre-publication mutators that
+  // used to be re-summed here on every call. Safe to read lock-free once
+  // published (Libraries::native_libs) because those mutators never run
+  // afterwards (asserted via _published), same invariant the old per-call
+  // rescan relied on.
+  //
+  // Deliberately excludes the build-id string: the background library
+  // refresher (Libraries::updateBuildIds) frees and replaces _build_id on
+  // already-published caches under _build_id_lock, which dump does not hold,
+  // so folding it into this running total would race. It is negligible
+  // (~tens of bytes per library) next to the symbol tables.
+  return _memory_usage;
 }
 
 void CodeCache::expand() {
@@ -200,6 +192,8 @@ void CodeCache::expand() {
 
   memcpy(new_blobs, old_blobs, _count * sizeof(CodeBlob));
 
+  // Delta is exactly the current (pre-doubling) capacity's worth of blobs.
+  _memory_usage += (long long)_capacity * sizeof(CodeBlob);
   _capacity *= 2;
   _blobs = new_blobs;
   delete[] old_blobs;
@@ -214,6 +208,7 @@ void CodeCache::add(const void *start, int length, const char *name,
   assert(!_published.load(std::memory_order_acquire) &&
          "add() on a published CodeCache races memoryUsage()");
   char *name_copy = NativeFunc::create(name, _lib_index);
+  _memory_usage += (long long)NativeFunc::allocSize(name);
   // Replace non-printable characters
   for (char *s = name_copy; *s != 0; s++) {
     if (*s < ' ')
@@ -478,6 +473,7 @@ void CodeCache::setDwarfTable(FrameDesc *table, int length, const FrameDesc &def
   // buffer to exactly `length` entries before returning it from table(), so
   // memoryUsage()'s length-based formula matches the real allocation with no
   // trim step needed here.
+  _memory_usage += (long long)length * sizeof(FrameDesc);
   _dwarf_table = table;
   _dwarf_table_length = length;
   _default_frame = &default_frame;

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

@@ -10,6 +10,7 @@
 #include "common.h"
 #include "counters.h"
 #include "dwarf.h"
+#include "nativeMem.h"
 #include "utils.h"
 
 #include <atomic>
@@ -164,6 +165,14 @@ class CodeCache {
   int _count;
   CodeBlob *_blobs;
 
+  // Running total of memoryUsage()'s formula, updated incrementally by the
+  // same pre-publication mutators (constructor, add(), expand(),
+  // setDwarfTable()) instead of recomputed by iterating _blobs. Safe because
+  // every contributing field is fixed before publication (see _published
+  // below) -- there is no point after construction-and-population where this
+  // total could be stale relative to a fresh recompute.
+  long long _memory_usage;
+
   // Set once the cache is registered into a CodeCacheArray (see markPublished()).
   // After that, memoryUsage() may be read lock-free from another thread (dump),
   // so the mutators that touch the fields it reads (add()/expand()/
@@ -328,6 +337,17 @@ class CodeCacheArray {
     } while (!__atomic_compare_exchange_n(&_reserved, &slot, slot + 1,
                                           true, __ATOMIC_RELAXED, __ATOMIC_RELAXED));
     assert(__atomic_load_n(&_libs[slot], __ATOMIC_RELAXED) == nullptr);
+    // Record this library's contribution to NM_NATIVE_SYMBOLS exactly once,
+    // right here at publication -- not via a periodic recompute-and-overwrite
+    // elsewhere. A cache that fails to publish (the overflow branch above) is
+    // simply never recorded, so there is no corresponding decrement to get
+    // right on the parse-failure/duplicate discard path in
+    // Symbols::parseLibraries (which deletes an unpublished cache directly).
+    // Because record() is a relaxed atomic add, concurrent publishers from
+    // different threads (or a publish racing Profiler::dump()'s read of the
+    // aggregate) can never clobber each other's contribution -- unlike the
+    // read-modify-write a periodic setLive(sum-of-everything) would need.
+    NativeMem::record(NM_NATIVE_SYMBOLS, lib->memoryUsage());
     // Mark published before the RELEASE store makes the pointer visible, so any
     // later add()/expand()/setDwarfTable() on this cache trips the assert (its
     // _blobs would then be read lock-free by memoryUsage() at dump time).
@@ -351,11 +371,11 @@ class CodeCacheArray {
     return __atomic_load_n(&_libs[index], __ATOMIC_ACQUIRE);
   }
 
-  // Sum the live memory of all registered libraries. Recomputed on demand
-  // (called only at dump time) so it reflects each library's fully-populated
-  // state at that point — the accurate per-library formula rather than a value
-  // cached at add() time. (Libraries are populated before being registered and
-  // not grown afterwards.) The array is append-only, so iterating the published
+  // Sum the live memory of all registered libraries. Each lib->memoryUsage()
+  // is now an O(1) read of a running total (see CodeCache::_memory_usage),
+  // not a rescan of its symbol table, so this whole sum is O(libraries) --
+  // still only called at dump time, but no longer the reason to avoid calling
+  // it more often. The array is append-only, so iterating the published
   // prefix is safe alongside concurrent add()s.
   size_t memoryUsage() const {
     size_t total = 0;

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

@@ -60,6 +60,20 @@ void Libraries::mangle(const char *name, char *buf, size_t size) {
 void Libraries::updateSymbols(bool kernel_symbols) {
   Symbols::parseLibraries(&_native_libs, kernel_symbols);
   LibraryPatcher::patch_libraries();
+  // NM_NATIVE_SYMBOLS accounting happens per-library, at the moment each
+  // CodeCache is published (CodeCacheArray::add(), codeCache.h) -- not here.
+  //
+  // A periodic recompute-and-overwrite from this function would have two
+  // problems. First, it would turn every dlopen refresh into an O(total
+  // symbols across every loaded library) rescan instead of O(new symbols
+  // only). Second, a dump()-triggered refresh (Profiler::dump()) can run
+  // concurrently with the background refresher thread's own refresh() --
+  // neither takes a common lock -- so whichever recompute-and-overwrite
+  // finishes last would win even if it started first with a smaller, stale
+  // total. The publish-time NativeMem::record() approach has neither
+  // problem: each publish is an O(1) atomic add of that one library's
+  // already-computed total, so there is nothing to rescan and nothing to
+  // race.
 }
 
 void Libraries::refresh() {

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

@@ -1889,8 +1889,18 @@ Error Profiler::check(Arguments &args) {
 
 void Profiler::updateNativeLibMemStats() {
   // CodeCache here is the profiler's native-symbol tables (not the JVM code
-  // cache). memoryUsage() is a recomputed gauge; read it once and publish the
-  // counters plus the NativeMem gauge (NATIVE_SYMBOLS) as absolutes.
+  // cache). memoryUsage() is now an O(1) read of each library's running
+  // total (see CodeCache::_memory_usage), not a rescan, so reading it here
+  // for the Counters:: mirrors below is cheap even though this itself is
+  // only called from stop()/dump().
+  //
+  // Deliberately does NOT also write NM_NATIVE_SYMBOLS here: that gauge is
+  // maintained incrementally at publish time (CodeCacheArray::add(), an
+  // atomic add per library). Overwriting it with a fresh recompute from this
+  // function -- which can run concurrently with the background refresher
+  // thread publishing a new library, since neither takes a common lock --
+  // would reintroduce exactly the stale-overwrite race the publish-time
+  // accounting was built to avoid.
   const CodeCacheArray& native_libs = _libs->native_libs();
   long long usage = (long long)native_libs.memoryUsage();
   Counters::set(CODECACHE_NATIVE_COUNT, native_libs.count());
@@ -1900,7 +1910,6 @@ void Profiler::updateNativeLibMemStats() {
   // behind the VM abstraction (0 on J9/Zing).
   Counters::set(CODECACHE_RUNTIME_STUBS_SIZE_BYTES,
                 JVMSupport::runtimeStubsMemoryUsage());
-  NativeMem::setLive(NM_NATIVE_SYMBOLS, usage);
 }
 
 Error Profiler::dump(const char *path, const int length) {