fix(jfr): skip emitting empty constant pools (#603)

Author: jbachorik

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

@@ -761,7 +761,8 @@ off_t Recording::finishChunk(bool end_recording, bool do_cleanup) {
   }
 
   off_t cpool_offset = lseek(_fd, 0, SEEK_CUR);
-  writeCpool(_buf);
+  int count_offset_in_cpool = 0;
+  int pool_count = writeCpool(_buf, &count_offset_in_cpool);
   flush(_buf);
 
   off_t cpool_end = lseek(_fd, 0, SEEK_CUR);
@@ -771,6 +772,13 @@ off_t Recording::finishChunk(bool end_recording, bool do_cleanup) {
   ssize_t result = pwrite(_fd, _buf->data(), 5, cpool_offset);
   (void)result;
 
+  // Patch the constant pool count placeholder (written as a 1-byte put8 in
+  // writeCpool). Done flush-safe via pwrite to the FILE offset, mirroring the
+  // size patch above: _buf has been flushed/reset, so _buf->data() is scratch.
+  _buf->put8(0, (char)pool_count);
+  result = pwrite(_fd, _buf->data(), 1, cpool_offset + count_offset_in_cpool);
+  (void)result;
+
   off_t chunk_end = lseek(_fd, 0, SEEK_CUR);
 
   // // Workaround for JDK-8191415: compute actual TSC frequency, in case JFR is
@@ -1359,15 +1367,24 @@ void Recording::writeNativeLibraries(Buffer *buf) {
   _recorded_lib_count = native_lib_count;
 }
 
-void Recording::writeCpool(Buffer *buf) {
+int Recording::writeCpool(Buffer *buf, int *count_offset_in_cpool) {
+  // Offset of the cpool start within the buffer. The header below is tiny and
+  // flush-free, so the placeholder offset captured relative to this start is a
+  // stable cpool-relative offset usable for a flush-safe back-patch by the
+  // caller (mirrors the cpool SIZE patch).
+  int cpool_start = buf->offset();
   buf->skip(5); // size will be patched later
   buf->putVar64(T_CPOOL);
   buf->putVar64(_start_ticks);
   buf->put8(0);
   buf->put8(0);
   buf->put8(1);
-  // constant pool count - bump each time a new pool is added
-  buf->put8(12);
+  // Constant pool count. We cannot precompute it: the Method/Class/Package/Symbol
+  // pools are only fully populated as a side effect of writeStackTraces/writeMethods
+  // (fillJavaMethodInfo), and empty variable pools are skipped entirely. Write a
+  // 1-byte placeholder here and back-patch it flush-safe in the caller.
+  *count_offset_in_cpool = buf->offset() - cpool_start;
+  buf->put8(0);
 
   // Profiler::rotateDictsAndRun() rotates the three dictionaries before this
   // path runs, so classMap()->standby() returns an old-active snapshot stable
@@ -1379,21 +1396,26 @@ void Recording::writeCpool(Buffer *buf) {
   // standby() captures the pre-rotation state which writeClasses extends.
   Lookup lookup(this, &_method_map, Profiler::instance()->classMap());
   lookup.initClassCache();
+  // CONSTANT pools: always non-empty, always emitted -> 5 sections.
+  // writeThreads always emits: it inserts _tid unconditionally before checking.
   writeFrameTypes(buf);
   writeThreadStates(buf);
   writeExecutionModes(buf);
-  writeThreads(buf);
-  writeStackTraces(buf, &lookup);
-  writeMethods(buf, &lookup);
-  writeClasses(buf, &lookup);
-  writePackages(buf, &lookup);
-  writeConstantPoolSection(buf, T_SYMBOL, &lookup._symbols);
-  writeConstantPoolSection(buf, T_STRING,
-                           Profiler::instance()->stringLabelMap()->standby());
-  writeConstantPoolSection(buf, T_ATTRIBUTE_VALUE,
-                           Profiler::instance()->contextValueMap()->standby());
   writeLogLevels(buf);
+  writeThreads(buf);
+  int pool_count = 5;
+  // VARIABLE pools: each returns 1 if emitted, 0 if empty (and thus skipped).
+  pool_count += writeStackTraces(buf, &lookup);
+  pool_count += writeMethods(buf, &lookup);
+  pool_count += writeClasses(buf, &lookup);
+  pool_count += writePackages(buf, &lookup);
+  pool_count += writeConstantPoolSection(buf, T_SYMBOL, &lookup._symbols);
+  pool_count += writeConstantPoolSection(
+      buf, T_STRING, Profiler::instance()->stringLabelMap()->standby());
+  pool_count += writeConstantPoolSection(
+      buf, T_ATTRIBUTE_VALUE, Profiler::instance()->contextValueMap()->standby());
   flushIfNeeded(buf);
+  return pool_count;
 }
 
 void Recording::writeFrameTypes(Buffer *buf) {
@@ -1466,7 +1488,7 @@ void Recording::writeThreads(Buffer *buf) {
   // We flush from old_index (the previous active set)
 
   std::unordered_set<int> threads;
-  threads.insert(_tid);
+  threads.insert(_tid);  // always present: the recording thread itself
 
   for (int i = 0; i < CONCURRENCY_LEVEL; ++i) {
     // Collect thread IDs from the fixed-size table into the main set
@@ -1512,14 +1534,22 @@ void Recording::writeThreads(Buffer *buf) {
   }
 }
 
-void Recording::writeStackTraces(Buffer *buf, Lookup *lookup) {
+int Recording::writeStackTraces(Buffer *buf, Lookup *lookup) {
   // Reset all referenced flags before processing
   for (MethodMap::iterator it = _method_map.begin(); it != _method_map.end(); ++it) {
     it->second._referenced = false;
   }
 
+  // Tracks how many traces were written so the empty pool can be skipped.
+  // Note: even with zero traces, the methods marking pass below must still run
+  // via processCallTraces, but no T_STACK_TRACE section is emitted in that case.
+  int trace_count = 0;
   // Use safe trace processing with guaranteed lifetime during callback execution
-  Profiler::instance()->processCallTraces([this, buf, lookup](const std::unordered_set<CallTrace*>& traces) {
+  Profiler::instance()->processCallTraces([this, buf, lookup, &trace_count](const std::unordered_set<CallTrace*>& traces) {
+    if (traces.empty()) {
+      return;
+    }
+    trace_count = (int)traces.size();
     buf->putVar64(T_STACK_TRACE);
     buf->putVar64(traces.size());
     for (std::unordered_set<CallTrace *>::const_iterator it = traces.begin();
@@ -1558,9 +1588,10 @@ void Recording::writeStackTraces(Buffer *buf, Lookup *lookup) {
       flushIfNeeded(buf);
     }
   });  // End of processCallTraces lambda
+  return trace_count > 0 ? 1 : 0;
 }
 
-void Recording::writeMethods(Buffer *buf, Lookup *lookup) {
+int Recording::writeMethods(Buffer *buf, Lookup *lookup) {
   MethodMap *method_map = lookup->_method_map;
 
   u32 marked_count = 0;
@@ -1571,6 +1602,10 @@ void Recording::writeMethods(Buffer *buf, Lookup *lookup) {
     }
   }
 
+  if (marked_count == 0) {
+    return 0;
+  }
+
   buf->putVar64(T_METHOD);
   buf->putVar64(marked_count);
   for (MethodMap::iterator it = method_map->begin(); it != method_map->end();
@@ -1587,9 +1622,10 @@ void Recording::writeMethods(Buffer *buf, Lookup *lookup) {
       flushIfNeeded(buf);
     }
   }
+  return 1;
 }
 
-void Recording::writeClasses(Buffer *buf, Lookup *lookup) {
+int Recording::writeClasses(Buffer *buf, Lookup *lookup) {
   DEBUG_ASSERT_NOT_IN_SIGNAL();
   std::map<u32, const char *> classes;
   // standby() returns the dump buffer — the stable snapshot captured by
@@ -1598,6 +1634,10 @@ void Recording::writeClasses(Buffer *buf, Lookup *lookup) {
   // cross-thread writers via waitForRefCountToClear() before returning.
   lookup->_classes->standby()->collect(classes);
 
+  if (classes.empty()) {
+    return 0;
+  }
+
   buf->putVar64(T_CLASS);
   buf->putVar64(classes.size());
   for (std::map<u32, const char *>::const_iterator it = classes.begin();
@@ -1610,12 +1650,17 @@ void Recording::writeClasses(Buffer *buf, Lookup *lookup) {
     buf->putVar64(0); // access flags
     flushIfNeeded(buf);
   }
+  return 1;
 }
 
-void Recording::writePackages(Buffer *buf, Lookup *lookup) {
+int Recording::writePackages(Buffer *buf, Lookup *lookup) {
   std::map<u32, const char *> packages;
   lookup->_packages.collect(packages);
 
+  if (packages.empty()) {
+    return 0;
+  }
+
   buf->putVar32(T_PACKAGE);
   buf->putVar32(packages.size());
   for (std::map<u32, const char *>::const_iterator it = packages.begin();
@@ -1624,10 +1669,14 @@ void Recording::writePackages(Buffer *buf, Lookup *lookup) {
     buf->putVar64(lookup->getSymbol(it->second));
     flushIfNeeded(buf);
   }
+  return 1;
 }
 
-void Recording::writeConstantPoolSection(
+int Recording::writeConstantPoolSection(
     Buffer *buf, JfrType type, std::map<u32, const char *> &constants) {
+  if (constants.empty()) {
+    return 0;
+  }
   flushIfNeeded(buf);
   buf->putVar64(type);
   buf->putVar64(constants.size());
@@ -1639,20 +1688,21 @@ void Recording::writeConstantPoolSection(
     buf->putVar64(it->first);
     buf->putUtf8(it->second, length);
   }
+  return 1;
 }
 
-void Recording::writeConstantPoolSection(Buffer *buf, JfrType type,
-                                         Dictionary *dictionary) {
+int Recording::writeConstantPoolSection(Buffer *buf, JfrType type,
+                                        Dictionary *dictionary) {
   std::map<u32, const char *> constants;
   dictionary->collect(constants);
-  writeConstantPoolSection(buf, type, constants);
+  return writeConstantPoolSection(buf, type, constants);
 }
 
-void Recording::writeConstantPoolSection(Buffer *buf, JfrType type,
-                                         StringDictionaryBuffer *buffer) {
+int Recording::writeConstantPoolSection(Buffer *buf, JfrType type,
+                                        StringDictionaryBuffer *buffer) {
   std::map<u32, const char *> constants;
   buffer->collect(constants);
-  writeConstantPoolSection(buf, type, constants);
+  return writeConstantPoolSection(buf, type, constants);
 }
 
 void Recording::writeLogLevels(Buffer *buf) {

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

@@ -287,30 +287,37 @@ class Recording {
   void writeJvmInfo(Buffer *buf);
   void writeSystemProperties(Buffer *buf);
   void writeNativeLibraries(Buffer *buf);
-  void writeCpool(Buffer *buf);
+  // Writes the cpool checkpoint. Returns the number of pool sections actually
+  // emitted (empty variable pools are skipped) and reports the byte offset of
+  // the pool-count placeholder within the cpool via *count_offset_in_cpool, so
+  // the caller can back-patch it flush-safe alongside the cpool size field.
+  int writeCpool(Buffer *buf, int *count_offset_in_cpool);
 
   void writeFrameTypes(Buffer *buf);
 
   void writeThreadStates(Buffer *buf);
 
   void writeExecutionModes(Buffer *buf);
+  // writeThreads always emits: _tid is inserted unconditionally so the thread
+  // pool is never empty. The following variable-pool writers return 1 if a
+  // section was emitted, 0 if the pool was empty and skipped.
   void writeThreads(Buffer *buf);
 
-  void writeStackTraces(Buffer *buf, Lookup *lookup);
+  int writeStackTraces(Buffer *buf, Lookup *lookup);
 
-  void writeMethods(Buffer *buf, Lookup *lookup);
+  int writeMethods(Buffer *buf, Lookup *lookup);
 
-  void writeClasses(Buffer *buf, Lookup *lookup);
+  int writeClasses(Buffer *buf, Lookup *lookup);
 
-  void writePackages(Buffer *buf, Lookup *lookup);
+  int writePackages(Buffer *buf, Lookup *lookup);
 
-  void writeConstantPoolSection(Buffer *buf, JfrType type,
-                                std::map<u32, const char *> &constants);
+  int writeConstantPoolSection(Buffer *buf, JfrType type,
+                               std::map<u32, const char *> &constants);
 
-  void writeConstantPoolSection(Buffer *buf, JfrType type,
-                                Dictionary *dictionary);
-  void writeConstantPoolSection(Buffer *buf, JfrType type,
-                                StringDictionaryBuffer *buffer);
+  int writeConstantPoolSection(Buffer *buf, JfrType type,
+                               Dictionary *dictionary);
+  int writeConstantPoolSection(Buffer *buf, JfrType type,
+                               StringDictionaryBuffer *buffer);
 
   void writeLogLevels(Buffer *buf);