Enable ASGCT by default on fairly safe J9 JDK versions

.github/scripts/prepare_reports.sh

@@ -1,8 +1,9 @@
-#!/usr/bin/env bash
+  #!/usr/bin/env bash
 
 set -e
 mkdir -p reports
 cp /tmp/hs_err* reports/ || true
+cp ddprof-test/javacore*.txt reports/ || true
 cp ddprof-test/build/hs_err* reports/ || true
 cp -r ddprof-lib/build/tmp reports/native_build || true
 cp -r ddprof-test/build/reports/tests reports/tests || true

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

@@ -15,6 +15,8 @@
  */
 
 #include "arguments.h"
+#include "vmEntry.h"
+
 #include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -342,6 +344,14 @@ Error Arguments::parse(const char *args) {
     _event = EVENT_CPU;
   }
 
+  if (VM::isOpenJ9()) {
+    if (_cstack == CSTACK_FP) {
+      // J9 is compiled without FP
+      //   switch to DWARF for better results
+      _cstack = CSTACK_DWARF;
+    }
+  }
+
   return Error::OK;
 }
 

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

@@ -20,7 +20,9 @@
 
 #include <jvmti.h>
 
+#include "log.h"
 #include "vmEntry.h"
+#include "vmStructs.h"
 
 #define JVMTI_EXT(f, ...) ((jvmtiError(*)(jvmtiEnv *, __VA_ARGS__))f)
 
@@ -54,25 +56,40 @@ class J9Ext {
 
 public:
   static bool can_use_ASGCT() {
+    // as of 21.0.6 the use of ASGCT will lead to almost immediate crash
+    //   or livelock on J9
     return (VM::java_version() == 8 && VM::java_update_version() >= 362) ||
            (VM::java_version() == 11 && VM::java_update_version() >= 18) ||
            (VM::java_version() == 17 && VM::java_update_version() >= 6) ||
-           (VM::java_version() >= 18);
-  }
-
-  static bool is_jmethodid_safe() {
-    return VM::java_version() == 8 ||
-           (VM::java_version() == 11 && VM::java_update_version() >= 23) ||
-           (VM::java_version() == 17 && VM::java_update_version() >= 11) ||
-           (VM::java_version() == 21 && VM::java_update_version() >= 3) ||
-           (VM::java_version() >= 22);
+           (VM::java_version() >= 18 && VM::java_version() < 21);
   }
 
   static bool is_jvmti_jmethodid_safe() {
     // only JDK 8 is safe to use jmethodID in JVMTI for deferred resolution
+    //   unless -XX:+KeepJNIIDs=true is provided
     return VM::java_version() == 8;
   }
 
+  static bool shouldUseAsgct() {
+    char *sampler = NULL;
+
+    jvmtiEnv *jvmti = VM::jvmti();
+    jvmti->GetSystemProperty("dd.profiling.ddprof.j9.sampler", &sampler);
+
+    bool asgct = true;
+    if (sampler != nullptr) {
+      if (!strncmp("asgct", sampler, 5)) {
+        asgct = true;
+      } else if (!strncmp("jvmti", sampler, 5)) {
+        asgct = false;
+      } else {
+        fprintf(stdout, "[ddprof] [WARN] Invalid J9 sampler: %s, supported values are [jvmti, asgct]", sampler);
+      }
+    }
+    jvmti->Deallocate((unsigned char *)sampler);
+    return asgct;
+  }
+
   static bool initialize(jvmtiEnv *jvmti, const void *j9thread_self);
 
   static int GetOSThreadID(jthread thread) {

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

@@ -109,6 +109,14 @@ Java_com_datadoghq_profiler_JavaProfiler_execute0(JNIEnv *env, jobject unused,
   return NULL;
 }
 
+extern "C" DLLEXPORT jstring JNICALL
+Java_com_datadoghq_profiler_JavaProfiler_getStatus0(JNIEnv* env, 
+                                                    jobject unused) {
+  char msg[2048];
+  int ret = Profiler::instance()->status((char*)msg, sizeof(msg) - 1);
+  return env->NewStringUTF(msg);
+}
+
 extern "C" DLLEXPORT jlong JNICALL
 Java_com_datadoghq_profiler_JavaProfiler_getSamples(JNIEnv *env,
                                                     jobject unused) {

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

@@ -7,6 +7,7 @@
 #include "profiler.h"
 #include "asyncSampleMutex.h"
 #include "context.h"
+#include "common.h"
 #include "counters.h"
 #include "ctimer.h"
 #include "dwarf.h"
@@ -985,11 +986,16 @@ Engine *Profiler::selectCpuEngine(Arguments &args) {
     return &noop_engine;
   } else if (args._cpu >= 0 || strcmp(args._event, EVENT_CPU) == 0) {
     if (VM::isOpenJ9()) {
-      if (!J9Ext::is_jvmti_jmethodid_safe()) {
-        Log::warn("Safe jmethodID access is not available on this JVM. Using "
-                  "CPU profiler on your own risk.");
+      if (!J9Ext::shouldUseAsgct() || !J9Ext::can_use_ASGCT()) {
+        if (!J9Ext::is_jvmti_jmethodid_safe()) {
+          fprintf(stderr, "[ddprof] [WARN] Safe jmethodID access is not available on this JVM. Using "
+                    "CPU profiler on your own risk. Use -XX:+KeepJNIIDs=true JVM "
+                    "flag to make access to jmethodIDs safe, if your JVM supports it\n");
+        }
+        TEST_LOG("J9[cpu]=jvmti");
+        return &j9_engine;
       }
-      return &j9_engine;
+      TEST_LOG("J9[cpu]=asgct");
     }
     return !ctimer.check(args)
                ? (Engine *)&ctimer
@@ -1012,14 +1018,17 @@ Engine *Profiler::selectWallEngine(Arguments &args) {
     return &noop_engine;
   }
   if (VM::isOpenJ9()) {
-    if (args._wallclock_sampler == JVMTI) {
+    if (args._wallclock_sampler == JVMTI || !J9Ext::shouldUseAsgct() || !J9Ext::can_use_ASGCT()) {
       if (!J9Ext::is_jvmti_jmethodid_safe()) {
-        Log::warn("Safe jmethodID access is not available on this JVM. Using "
-                  "wallclock profiler on your own risk.");
+        fprintf(stderr, "[ddprof] [WARN] Safe jmethodID access is not available on this JVM. Using "
+                  "wallclock profiler on your own risk. Use -XX:+KeepJNIIDs=true JVM "
+                  "flag to make access to jmethodIDs safe, if your JVM supports it\n");
       }
       j9_engine.sampleIdleThreads();
+      TEST_LOG("J9[wall]=jvmti");
       return (Engine *)&j9_engine;
     } else {
+      TEST_LOG("J9[wall]=asgct");
       return (Engine *)&wall_asgct_engine;
     }
   }
@@ -1474,3 +1483,16 @@ int Profiler::lookupClass(const char *key, size_t length) {
   // unable to lookup the class
   return -1;
 }
+
+int Profiler::status(char* status, int max_len) {
+  return snprintf(status, max_len,
+    "== Java-Profiler Status ===\n"
+    " Running          : %s\n"
+    " CPU Engine       : %s\n"
+    " WallClock Engine : %s\n"
+    " Allocations      : %s\n",
+    _state == RUNNING ? "true" : "false",
+    _cpu_engine != nullptr ? _cpu_engine->name() : "None",
+    _wall_engine != nullptr ? _wall_engine->name() : "None",
+    _alloc_engine != nullptr ? _alloc_engine->name() : "None");
+}

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

@@ -135,6 +135,7 @@ class Profiler {
   void updateJavaThreadNames();
   void updateNativeThreadNames();
   void mangle(const char *name, char *buf, size_t size);
+
   Engine *selectCpuEngine(Arguments &args);
   Engine *selectWallEngine(Arguments &args);
   Engine *selectAllocEngine(Arguments &args);
@@ -169,6 +170,8 @@ class Profiler {
     return _instance;
   }
 
+  int status(char* status, int max_len);
+
   const char* cstack() {
     switch (_cstack) {
       case CSTACK_DEFAULT: return "default";

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

@@ -383,10 +383,6 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) {
       *flag_addr = 1;
     }
   }
-  char *flag_addr = (char *)JVMFlag::find("KeepJNIIDs", {JVMFlag::Type::Bool});
-  if (flag_addr != NULL) {
-    *flag_addr = 1;
-  }
 
   // if the user sets -XX:+UseAdaptiveGCBoundary we will just disable the
   // profiler to avoid the risk of crashing flag was made obsolete (inert) in 15

ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java

@@ -167,6 +167,10 @@ public String getVersion() {
         }
     }
 
+    public String getStatus() {
+        return getStatus0();
+    }
+
     /**
      * Execute an agent-compatible profiling command -
      * the comma-separated list of arguments described in arguments.cpp
@@ -472,4 +476,6 @@ public Map<String, Long> getDebugCounters() {
     private static native long tscFrequency0();
 
     private static native void mallocArenaMax0(int max);
+
+    private static native String getStatus0();
 }

ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProcessProfilerTest.java

@@ -0,0 +1,94 @@
+package com.datadoghq.profiler;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+
+public abstract class AbstractProcessProfilerTest {
+    protected final boolean launch(String target, List<String> jvmArgs, String commands, Function<String, Boolean> onStdoutLine, Function<String, Boolean> onStderrLine) throws Exception {
+        String javaHome = System.getenv("JAVA_TEST_HOME");
+        if (javaHome == null) {
+            javaHome = System.getenv("JAVA_HOME");
+        }
+        if (javaHome == null) {
+            javaHome = System.getProperty("java.home");
+        }
+        assertNotNull(javaHome);
+
+        List<String> args = new ArrayList<>();
+        args.add(javaHome + "/bin/java");
+        args.addAll(jvmArgs);
+        args.add("-cp");
+        args.add(System.getProperty("java.class.path"));
+        args.add(ExternalLauncher.class.getName());
+        args.add(target);
+        if (commands != null && !commands.isEmpty()) {
+            args.add(commands);
+        }
+
+        ProcessBuilder pb = new ProcessBuilder(args);
+        Process p = pb.start();
+        Thread stdoutReader = new Thread(() -> {
+            Function<String, Boolean> lineProcessor = onStdoutLine != null ? onStdoutLine : l -> true;
+            try (BufferedReader br = new BufferedReader(new InputStreamReader(p.getInputStream()))) {
+                String line;
+                while ((line = br.readLine()) != null) {
+                    System.out.println("[out] " + line);
+                    if (!lineProcessor.apply(line)) {
+                        try {
+                            p.getOutputStream().write(1);
+                            p.getOutputStream().flush();
+                        } catch (IOException ignored) {
+                        }
+                    } else {
+                        if (line.contains("[ready]")) {
+                            p.getOutputStream().write(1);
+                            p.getOutputStream().flush();
+                        }
+                    }
+                }
+            } catch (IOException ignored) {
+            } catch (Exception e) {
+                throw new RuntimeException(e);
+            }
+        }, "stdout-reader");
+        Thread stderrReader = new Thread(() -> {
+            Function<String, Boolean> lineProcessor = onStderrLine != null ? onStderrLine : l -> true;
+            try (BufferedReader br = new BufferedReader(new InputStreamReader(p.getErrorStream()))) {
+                String line;
+                while ((line = br.readLine()) != null) {
+                    System.out.println("[err] " + line);
+                    if (!lineProcessor.apply(line)) {
+                        try {
+                            p.getOutputStream().write(1);
+                            p.getOutputStream().flush();
+                        } catch (IOException ignored) {
+                        }
+                    }
+                }
+            } catch (IOException ignored) {
+            } catch (Exception e) {
+                throw new RuntimeException(e);
+            }
+        }, "stderr-reader");
+
+        stdoutReader.setDaemon(true);
+        stderrReader.setDaemon(true);
+
+        stdoutReader.start();
+        stderrReader.start();
+
+        boolean val = p.waitFor(10, TimeUnit.SECONDS);
+        if (!val) {
+            p.destroyForcibly();
+        }
+        return val;
+    }
+}

ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java

@@ -2,13 +2,27 @@
 
 public class ExternalLauncher {
     public static void main(String[] args) throws Exception {
-        if (args.length != 1) {
-            throw new RuntimeException();
-        }
-        if (args[0].equals("library")) {
-            JVMAccess.getInstance();
-        } else if (args[0].equals("profiler")) {
-            JavaProfiler.getInstance();
+        try {
+            if (args.length < 1) {
+                throw new RuntimeException();
+            }
+            if (args[0].equals("library")) {
+                JVMAccess.getInstance();
+            } else if (args[0].equals("profiler")) {
+                JavaProfiler instance = JavaProfiler.getInstance();
+                if (args.length == 2) {
+                    String commands = args[1];
+                    if (!commands.isEmpty()) {
+                        instance.execute(commands);
+                    }
+                }
+            }
+        } finally {
+            System.out.println("[ready]");
+            System.out.flush();
+            System.err.flush();
         }
+        // wait for signal to exit
+        System.in.read();
     }
 }