Bound class-map growth to fix reliability-chaos OOMs

.gitlab/reliability/chaos_check.sh

@@ -123,6 +123,11 @@ case $ALLOCATOR in
   tcmalloc)
     echo "Running with tcmalloc"
     export LD_PRELOAD=$(find /usr/lib/ -name 'libtcmalloc_minimal.so.4')
+    # thread-churn/dump-storm antagonists cycle many short-lived threads;
+    # tcmalloc's defaults are slow to return their per-thread caches to the
+    # OS, which was inflating container RSS past the OOM limit on aarch64.
+    export TCMALLOC_RELEASE_RATE=10
+    export TCMALLOC_AGGRESSIVE_DECOMMIT=1
     ;;
   jemalloc)
     echo "Running with jemalloc"

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

@@ -42,6 +42,7 @@
   X(DICTIONARY_ENDPOINTS_ARENA_WASTE_BYTES, "dictionary_endpoints_arena_waste_bytes") \
   X(DICTIONARY_CONTEXT_ARENA_WASTE_BYTES, "dictionary_context_arena_waste_bytes")     \
   X(DICTIONARY_DRAIN_TIMEOUTS, "dictionary_drain_timeouts")                    \
+  X(CLASS_MAP_AT_CAPACITY, "class_map_at_capacity_drops")                      \
   X(CONTEXT_STORAGE_BYTES, "context_storage_bytes")                            \
   X(CONTEXT_STORAGE_PAGES, "context_storage_pages")                            \
   X(CONTEXT_BOUNDS_MISS_INITS, "context_bounds_miss_inits")                    \

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

@@ -68,7 +68,7 @@ SharedLineNumberTable::~SharedLineNumberTable() {
 
 void Lookup::fillNativeMethodInfo(MethodInfo *mi, const char *name,
                                   const char *lib_name) {
-  mi->_class = _classes->lookupDuringDump("", 0);
+  mi->_class = _classes->lookupDuringDump("", 0, Profiler::maxClassMapSize());
   // TODO return the library name once we figured out how to cooperate with the
   // backend
   //        if (lib_name == NULL) {
@@ -117,7 +117,8 @@ void Lookup::fillNativeMethodInfo(MethodInfo *mi, const char *name,
 
 void Lookup::fillRemoteFrameInfo(MethodInfo *mi, const RemoteFrameInfo *rfi) {
   // Store build-id in the class name field
-  mi->_class = _classes->lookupDuringDump(rfi->build_id, strlen(rfi->build_id));
+  mi->_class = _classes->lookupDuringDump(rfi->build_id, strlen(rfi->build_id),
+                                           Profiler::maxClassMapSize());
 
   // Store PC offset in hex format in the signature field
   char offset_hex[32];
@@ -269,35 +270,40 @@ void Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method,
                      "Ljdk/internal/reflect/GeneratedConstructorAccessor")) {
         class_name_id = _classes->lookupDuringDump(
             "jdk/internal/reflect/GeneratedConstructorAccessor",
-            strlen("jdk/internal/reflect/GeneratedConstructorAccessor"));
+            strlen("jdk/internal/reflect/GeneratedConstructorAccessor"),
+            Profiler::maxClassMapSize());
         method_name_id =
             _symbols.lookup("Object "
                             "jdk.internal.reflect.GeneratedConstructorAccessor."
                             "newInstance(Object[])");
         method_sig_id = _symbols.lookup(method_sig);
       } else if (has_prefix(class_name,
                             "Lsun/reflect/GeneratedConstructorAccessor")) {
-        class_name_id =
-            _classes->lookupDuringDump("sun/reflect/GeneratedConstructorAccessor",
-                                       strlen("sun/reflect/GeneratedConstructorAccessor"));
+        class_name_id = _classes->lookupDuringDump(
+            "sun/reflect/GeneratedConstructorAccessor",
+            strlen("sun/reflect/GeneratedConstructorAccessor"),
+            Profiler::maxClassMapSize());
         method_name_id = _symbols.lookup(
             "Object "
             "sun.reflect.GeneratedConstructorAccessor.newInstance(Object[])");
         method_sig_id = _symbols.lookup(method_sig);
       } else if (has_prefix(class_name,
                             "Ljdk/internal/reflect/GeneratedMethodAccessor")) {
-        class_name_id =
-            _classes->lookupDuringDump("jdk/internal/reflect/GeneratedMethodAccessor",
-                                       strlen("jdk/internal/reflect/GeneratedMethodAccessor"));
+        class_name_id = _classes->lookupDuringDump(
+            "jdk/internal/reflect/GeneratedMethodAccessor",
+            strlen("jdk/internal/reflect/GeneratedMethodAccessor"),
+            Profiler::maxClassMapSize());
         method_name_id =
             _symbols.lookup("Object "
                             "jdk.internal.reflect.GeneratedMethodAccessor."
                             "invoke(Object, Object[])");
         method_sig_id = _symbols.lookup(method_sig);
       } else if (has_prefix(class_name,
                             "Lsun/reflect/GeneratedMethodAccessor")) {
-        class_name_id = _classes->lookupDuringDump("sun/reflect/GeneratedMethodAccessor",
-                                                    strlen("sun/reflect/GeneratedMethodAccessor"));
+        class_name_id = _classes->lookupDuringDump(
+            "sun/reflect/GeneratedMethodAccessor",
+            strlen("sun/reflect/GeneratedMethodAccessor"),
+            Profiler::maxClassMapSize());
         method_name_id = _symbols.lookup(
             "Object sun.reflect.GeneratedMethodAccessor.invoke(Object, "
             "Object[])");
@@ -308,30 +314,38 @@ void Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method,
         // we want to normalise to java/lang/invoke/LambdaForm$MH,
         // java/lang/invoke/LambdaForm$DMH, java/lang/invoke/LambdaForm$BMH,
         if (has_prefix(class_name + lambdaFormPrefixLength, "MH")) {
-          class_name_id = _classes->lookupDuringDump("java/lang/invoke/LambdaForm$MH",
-                                                      strlen("java/lang/invoke/LambdaForm$MH"));
+          class_name_id = _classes->lookupDuringDump(
+              "java/lang/invoke/LambdaForm$MH",
+              strlen("java/lang/invoke/LambdaForm$MH"),
+              Profiler::maxClassMapSize());
         } else if (has_prefix(class_name + lambdaFormPrefixLength, "BMH")) {
-          class_name_id = _classes->lookupDuringDump("java/lang/invoke/LambdaForm$BMH",
-                                                      strlen("java/lang/invoke/LambdaForm$BMH"));
+          class_name_id = _classes->lookupDuringDump(
+              "java/lang/invoke/LambdaForm$BMH",
+              strlen("java/lang/invoke/LambdaForm$BMH"),
+              Profiler::maxClassMapSize());
         } else if (has_prefix(class_name + lambdaFormPrefixLength, "DMH")) {
-          class_name_id = _classes->lookupDuringDump("java/lang/invoke/LambdaForm$DMH",
-                                                      strlen("java/lang/invoke/LambdaForm$DMH"));
+          class_name_id = _classes->lookupDuringDump(
+              "java/lang/invoke/LambdaForm$DMH",
+              strlen("java/lang/invoke/LambdaForm$DMH"),
+              Profiler::maxClassMapSize());
         } else {
           // don't recognise the suffix, so don't normalise
           class_name_id = _classes->lookupDuringDump(
-              normalized_class_name, normalized_class_name_len);
+              normalized_class_name, normalized_class_name_len,
+              Profiler::maxClassMapSize());
         }
         method_name_id = _symbols.lookup(method_name);
         method_sig_id = _symbols.lookup(method_sig);
       } else {
-        class_name_id = _classes->lookupDuringDump(normalized_class_name,
-                                                   normalized_class_name_len);
+        class_name_id = _classes->lookupDuringDump(
+            normalized_class_name, normalized_class_name_len,
+            Profiler::maxClassMapSize());
         method_name_id = _symbols.lookup(method_name);
         method_sig_id = _symbols.lookup(method_sig);
       }
     } else {
       Counters::increment(JMETHODID_SKIPPED);
-      class_name_id = _classes->lookupDuringDump("", 0);
+      class_name_id = _classes->lookupDuringDump("", 0, Profiler::maxClassMapSize());
       method_name_id = _symbols.lookup("jvmtiError");
       method_sig_id = _symbols.lookup("()L;");
     }
@@ -426,7 +440,7 @@ bool Lookup::resolveVTableReceiver(VMSymbol *sym, char *buf, size_t bufsize,
   // the stack frame's class_id absent from this chunk's class pool.
   // (Plain lookup() remains correct for non-dump callers — e.g. Profiler::
   // lookupClass on JVM threads — where the next rotate() will propagate.)
-  u32 class_id = _classes->lookupDuringDump(buf, len);
+  u32 class_id = _classes->lookupDuringDump(buf, len, Profiler::maxClassMapSize());
   // Apply synthetic-accessor/LambdaForm normalisation so that the many
   // distinct names HotSpot generates for these families (..Accessor1234,
   // LambdaForm$MH/0x...) collapse to one bucket each in the JFR class pool.
@@ -436,32 +450,32 @@ bool Lookup::resolveVTableReceiver(VMSymbol *sym, char *buf, size_t bufsize,
   if (has_prefix_n(buf, len,
                    "jdk/internal/reflect/GeneratedConstructorAccessor")) {
     static const char kName[] = "jdk/internal/reflect/GeneratedConstructorAccessor";
-    class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1);
+    class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1, Profiler::maxClassMapSize());
   } else if (has_prefix_n(buf, len, "sun/reflect/GeneratedConstructorAccessor")) {
     static const char kName[] = "sun/reflect/GeneratedConstructorAccessor";
-    class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1);
+    class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1, Profiler::maxClassMapSize());
   } else if (has_prefix_n(buf, len,
                           "jdk/internal/reflect/GeneratedMethodAccessor")) {
     static const char kName[] = "jdk/internal/reflect/GeneratedMethodAccessor";
-    class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1);
+    class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1, Profiler::maxClassMapSize());
   } else if (has_prefix_n(buf, len, "sun/reflect/GeneratedMethodAccessor")) {
     static const char kName[] = "sun/reflect/GeneratedMethodAccessor";
-    class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1);
+    class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1, Profiler::maxClassMapSize());
   } else if (has_prefix_n(buf, len, "java/lang/invoke/LambdaForm$")) {
     size_t prefix_len = strlen("java/lang/invoke/LambdaForm$");
     const char *suffix = buf + prefix_len;
     size_t suffix_len = len - prefix_len;
     if (suffix_len >= 2 && suffix[0] == 'M' && suffix[1] == 'H') {
       static const char kName[] = "java/lang/invoke/LambdaForm$MH";
-      class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1);
+      class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1, Profiler::maxClassMapSize());
     } else if (suffix_len >= 3 && suffix[0] == 'B' && suffix[1] == 'M' &&
                suffix[2] == 'H') {
       static const char kName[] = "java/lang/invoke/LambdaForm$BMH";
-      class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1);
+      class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1, Profiler::maxClassMapSize());
     } else if (suffix_len >= 3 && suffix[0] == 'D' && suffix[1] == 'M' &&
                suffix[2] == 'H') {
       static const char kName[] = "java/lang/invoke/LambdaForm$DMH";
-      class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1);
+      class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1, Profiler::maxClassMapSize());
     }
   }
   *out_class_id = class_id;
@@ -485,7 +499,7 @@ u32 Lookup::resolveVTableReceiverCached(void *sym) {
     // marker instead of an empty class name (which is indistinguishable
     // from a parser/encoder error downstream).
     static const char kName[] = "<unresolved_vtable_receiver>";
-    class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1);
+    class_id = _classes->lookupDuringDump(kName, sizeof(kName) - 1, Profiler::maxClassMapSize());
   }
   _vtable_receiver_cache[sym] = class_id;
   return class_id;

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

@@ -117,12 +117,19 @@ void LivenessTracker::flush_table(std::set<int> *tracked_thread_ids) {
       env->DeleteLocalRef(clz);
       jniExceptionCheck(env);
       const char *name = env->GetStringUTFChars(name_str, nullptr);
-      event._id = name != nullptr
-                      ? Profiler::instance()->lookupClass(name, strlen(name))
-                      : 0;
+      int class_id = name != nullptr
+                         ? Profiler::instance()->lookupClass(name, strlen(name))
+                         : 0;
       env->ReleaseStringUTFChars(name_str, name);
 
-      Profiler::instance()->recordDeferredSample(_table[i].tid, _table[i].call_trace_id, BCI_LIVENESS, &event);
+      // lookupClass() returns -1 when the class map is at capacity; do not
+      // assign it to the u32 event id (it would wrap to 0xFFFFFFFF and
+      // corrupt liveness attribution in the JFR output) — drop the sample
+      // instead, matching ObjectSampler's convention for the same failure.
+      if (class_id >= 0) {
+        event._id = class_id;
+        Profiler::instance()->recordDeferredSample(_table[i].tid, _table[i].call_trace_id, BCI_LIVENESS, &event);
+      }
     }
 
     env->DeleteLocalRef(ref);

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

@@ -1832,10 +1832,16 @@ void Profiler::shutdown(Arguments &args) {
 }
 
 int Profiler::lookupClass(const char *key, size_t length) {
-  // StringDictionary::lookup() is internally thread-safe via _accepting +
-  // RefCountGuard; no external lock required (unlike the old Dictionary).
-  u32 id = _class_map.lookup(key, length);
-  return id != 0 ? static_cast<int>(id) : -1;
+  // StringDictionary::bounded_lookup() is internally thread-safe via
+  // _accepting + RefCountGuard; no external lock required. Bounded to
+  // MAX_CLASS_MAP_SIZE so unbounded distinct-class churn cannot grow this
+  // dictionary for the lifetime of the process.
+  u32 id = _class_map.bounded_lookup(key, length, MAX_CLASS_MAP_SIZE);
+  if (id == 0) {
+    Counters::increment(CLASS_MAP_AT_CAPACITY);
+    return -1;
+  }
+  return static_cast<int>(id);
 }
 
 int Profiler::status(char* status, int max_len) {

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

@@ -81,6 +81,15 @@ class alignas(alignof(SpinLock)) Profiler {
   NotifyClassUnloadedFunc _notify_class_unloaded_func;
   // --
 
+  // Caps the number of distinct class names _class_map will ever hold.
+  // rotate()/clearStandby() carry the full accumulated set forward on every
+  // JFR chunk (only clearAll() on profiler restart truly resets it), so
+  // without a cap a workload with unbounded distinct-class churn (e.g. heavy
+  // dynamic class generation) grows this dictionary for the lifetime of the
+  // process. 256K distinct classes is far beyond any real application's
+  // class count.
+  static const int MAX_CLASS_MAP_SIZE = 1 << 18;
+
   ThreadInfo _thread_info;
   StringDictionary _class_map{1};
   StringDictionary _string_label_map{2};
@@ -262,6 +271,10 @@ class alignas(alignof(SpinLock)) Profiler {
   Engine *wallEngine() { return _wall_engine; }
 
   StringDictionary *classMap() { return &_class_map; }
+  // Same cap lookupClass() enforces for sample-time inserts; dump-time
+  // insertions (Lookup::_classes->lookupDuringDump in flightRecorder.cpp)
+  // must use this too so _class_map cannot grow past it via that path.
+  static int maxClassMapSize() { return MAX_CLASS_MAP_SIZE; }
   SharedLockGuard classMapSharedGuard() { return SharedLockGuard(&_class_map_lock); }
   StringDictionary *stringLabelMap() { return &_string_label_map; }
   StringDictionary *contextValueMap() { return &_context_value_map; }

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

@@ -12,6 +12,7 @@
 #include "tripleBuffer.h"
 #include "arch.h"
 #include <atomic>
+#include <climits>
 #include <map>
 #include <stddef.h>
 #include <stdlib.h>
@@ -550,7 +551,11 @@ class StringDictionary {
     // Resolve a key during the dump phase.  Safe to call from the dump thread
     // after rotate(); must NOT be called from signal handlers or concurrently
     // with another lookupDuringDump call.
-    u32 lookupDuringDump(const char* key, size_t len) {
+    // size_limit bounds new insertions the same way bounded_lookup() does:
+    // once active->size() reaches size_limit, previously-known keys still
+    // resolve (dump/active hits above), but a genuinely new key returns 0
+    // instead of growing the dictionary further.
+    u32 lookupDuringDump(const char* key, size_t len, int size_limit = INT_MAX) {
         StringDictionaryBuffer* dump = _rot.dumpBuffer();
 
         u32 id = dump->lookup(key, len);
@@ -571,6 +576,7 @@ class StringDictionary {
             StringDictionaryBuffer* active = _rot.active();
             RefCountGuard guard(active);
             if (!guard.isActive()) return 0;
+            if (active->size() >= size_limit) return 0;
             u32 new_id = nextId();
             new_id = active->insert_with_id(key, len, new_id);
             if (new_id != 0) dump->insert_with_id(key, len, new_id);

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

@@ -210,6 +210,54 @@ TEST_F(LivenessTrackerTest, MultipleResizeOperationsMaintainCorrectCapacity) {
     EXPECT_EQ(mock.getCapacity(), 128);
 }
 
+/**
+ * Mock structure to test the flush_table id-assignment guard: Profiler::lookupClass()
+ * returns an int (-1 on class-map-at-capacity), but Event::_id is a u32. Assigning -1
+ * directly would wrap to 0xFFFFFFFF and corrupt liveness attribution, so flush_table
+ * must drop the sample instead. Mirrors ObjectSampler's convention for the same
+ * lookupClass() failure mode.
+ */
+struct FlushTableIdGuardMock {
+    bool recorded = false;
+    uint32_t recorded_id = 0;
+
+    // Correct behavior (after the fix): only assign/record when class_id >= 0.
+    void applyGuarded(int class_id) {
+        if (class_id >= 0) {
+            recorded = true;
+            recorded_id = static_cast<uint32_t>(class_id);
+        }
+    }
+
+    // Pre-fix behavior: unconditionally assigns class_id to the u32 event id.
+    void applyUnguarded(int class_id) {
+        recorded = true;
+        recorded_id = static_cast<uint32_t>(class_id);
+    }
+};
+
+TEST_F(LivenessTrackerTest, NegativeClassIdSampleIsDropped) {
+    FlushTableIdGuardMock mock;
+    mock.applyGuarded(-1);
+    EXPECT_FALSE(mock.recorded);
+}
+
+TEST_F(LivenessTrackerTest, NonNegativeClassIdSampleIsRecorded) {
+    FlushTableIdGuardMock mock;
+    mock.applyGuarded(42);
+    EXPECT_TRUE(mock.recorded);
+    EXPECT_EQ(42u, mock.recorded_id);
+}
+
+TEST_F(LivenessTrackerTest, UnguardedNegativeClassIdWrapsToMaxU32) {
+    // Documents the bug the guard prevents: without it, -1 wraps to 0xFFFFFFFF
+    // when narrowed to the u32 event id.
+    FlushTableIdGuardMock mock;
+    mock.applyUnguarded(-1);
+    EXPECT_TRUE(mock.recorded);
+    EXPECT_EQ(0xFFFFFFFFu, mock.recorded_id);
+}
+
 /**
  * Test that verifies capacity never exceeds max_cap during resize operations.
  */