Remove jvmti wallclock sampler for none J9 VMs (#419)

Author: zhengyu123

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

@@ -54,7 +54,6 @@ static void (*orig_busHandler)(int signo, siginfo_t *siginfo, void *ucontext);
 static Engine noop_engine;
 static PerfEvents perf_events;
 static WallClockASGCT wall_asgct_engine;
-static WallClockJVMTI wall_jvmti_engine;
 static J9WallClock j9_engine;
 static ITimer itimer;
 static CTimer ctimer;
@@ -1224,7 +1223,8 @@ Engine *Profiler::selectWallEngine(Arguments &args) {
   }
   switch (args._wallclock_sampler) {
         case JVMTI:
-            return (Engine*)&wall_jvmti_engine;
+            fprintf(stderr, "[ddprof] [WARN] JVMTI wallclock is not available on this JVM, fallback to ASGCT wallclock\n");
+            [[fallthrough]];
         case ASGCT:
         default:
             return (Engine*)&wall_asgct_engine;

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

@@ -154,122 +154,6 @@ void WallClockASGCT::initialize(Arguments& args) {
   OS::installSignalHandler(SIGVTALRM, sharedSignalHandler);
 }
 
-/* This method is extremely racy!
- * Thread references, that are returned from JVMTI::GetAllThreads(), only guarantee thread objects
- * are not collected by GCs, they don't prevent threads from exiting.
- * We have to be extremely careful when accessing thread's data, so it may not be valid.
- */
-void WallClockJVMTI::timerLoop() {
-    // Check for enablement before attaching/dettaching the current thread
-  if (!isEnabled()) {
-    return;
-  }
-
-  jvmtiEnv* jvmti = VM::jvmti();
-  if (jvmti == nullptr) {
-    return;
-  }
-
-  // Notice:
-  // We want to cache threads that are captured by collectThread(), so that we can
-  // clean them up in cleanThreadRefs().
-  // The approach is not ideal, but it is cleaner than cleaning individual thread
-  // during filtering phases.
-  jint threads_count = 0;
-  jthread* threads_ptr = nullptr;
-
-  // Attach to JVM as the first step
-  VM::attachThread("Datadog Profiler Wallclock Sampler");
-  auto collectThreads = [&](std::vector<ThreadEntry>& threads) {
-      jvmtiEnv* jvmti = VM::jvmti();
-      if (jvmti == nullptr) {
-          return;
-      }
-
-      if (jvmti->GetAllThreads(&threads_count, &threads_ptr) != JVMTI_ERROR_NONE ||
-        threads_count == 0) {
-        return;
-      }
-
-      JNIEnv* jni = VM::jni();
-
-      ThreadFilter* threadFilter = Profiler::instance()->threadFilter();
-      bool do_filter = threadFilter->enabled();
-      int self = OS::threadId();
-
-      // If filtering is enabled, collect the filtered TIDs first
-      std::vector<int> filtered_tids;
-      if (do_filter) {
-          Profiler::instance()->threadFilter()->collect(filtered_tids);
-          // Sort the TIDs for efficient lookup
-          std::sort(filtered_tids.begin(), filtered_tids.end());
-      }
-
-      for (int i = 0; i < threads_count; i++) {
-        jthread thread = threads_ptr[i];
-        if (thread != nullptr) {
-          VMThread* nThread = VMThread::fromJavaThread(jni, thread);
-          if (nThread == nullptr) {
-            continue;
-          }
-          int tid = nThread->osThreadId();
-          if (tid != self && (!do_filter ||
-               // Use binary search to efficiently find if tid is in filtered_tids
-               std::binary_search(filtered_tids.begin(), filtered_tids.end(), tid))) {
-            threads.push_back({nThread, thread, tid});
-          }
-        }
-      }
-    };
-
-  auto sampleThreads = [&](ThreadEntry& thread_entry, int& num_failures, int& threads_already_exited, int& permission_denied) {
-    static jint max_stack_depth = (jint)Profiler::instance()->max_stack_depth();
-
-    ExecutionEvent event;
-    VMThread* vm_thread = thread_entry.native;
-    int raw_thread_state = vm_thread->state();
-    bool is_initialized = raw_thread_state >= JVMJavaThreadState::_thread_in_native &&
-                          raw_thread_state < JVMJavaThreadState::_thread_max_state;
-    OSThreadState state = OSThreadState::UNKNOWN;
-    ExecutionMode mode = ExecutionMode::UNKNOWN;
-    if (vm_thread == nullptr || !is_initialized) {
-        return false;
-    }
-    OSThreadState os_state = vm_thread->osThreadState();
-    if (os_state == OSThreadState::TERMINATED) {
-      return false;
-    } else if (os_state == OSThreadState::UNKNOWN) {
-      state = OSThreadState::RUNNABLE;
-    } else {
-      state = os_state;
-    }
-    mode = getThreadExecutionMode();
-
-    event._thread_state = state;
-    event._execution_mode = mode;
-    event._weight =  1;
-
-    Profiler::instance()->recordJVMTISample(1, thread_entry.tid, thread_entry.java, BCI_WALL, &event, false);
-    return true;
-  };
-
-  auto cleanThreadRefs = [&]() {
-      JNIEnv* jni = VM::jni();
-      for (jint index = 0; index < threads_count; index++) {
-        jni->DeleteLocalRef(threads_ptr[index]);
-      }
-      jvmti->Deallocate((unsigned char*)threads_ptr);
-      threads_ptr = nullptr;
-      threads_count = 0;
-  };
-
-  timerLoopCommon<ThreadEntry>(collectThreads, sampleThreads, cleanThreadRefs, _reservoir_size, _interval);
-
-
-  // Don't forget to detach the thread
-  VM::detachThread();
-}
-
 void WallClockASGCT::timerLoop() {
     // todo: re-allocating the vector every time is not efficient
     auto collectThreads = [&](std::vector<int>& tids) {

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

@@ -155,19 +155,4 @@ class WallClockASGCT : public BaseWallClock {
     }
 };
 
-class WallClockJVMTI : public BaseWallClock {
-  private:
-    void timerLoop() override;
-  public:
-    struct ThreadEntry {
-        VMThread* native;
-        jthread java;
-        int tid;
-    };
-    WallClockJVMTI() : BaseWallClock() {}
-    const char* name() override {
-    return "WallClock (JVMTI)";
-  }
-};
-
 #endif // _WALLCLOCK_H

ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/BaseContextWallClockTest.java

@@ -47,6 +47,9 @@ void before() {
     }
 
     void after() throws InterruptedException {
+        if (executor == null) {
+            return;
+        }
         executor.shutdownNow();
         boolean terminated = executor.awaitTermination(30, TimeUnit.SECONDS);
         if (!terminated) {

ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedContextWallClockTest.java

@@ -20,6 +20,11 @@ protected void after() throws InterruptedException {
         base.after();
     }
 
+    @Override
+    protected boolean isPlatformSupported() {
+        return Platform.isJ9();
+    }
+
     @RetryingTest(5)
     public void test() throws ExecutionException, InterruptedException {
         // thread local handshake available only since Java 15

ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedWallClockThreadFilterTest.java

@@ -1,6 +1,12 @@
 package com.datadoghq.profiler.wallclock;
 
+import com.datadoghq.profiler.Platform;
+
 public class JvmtiBasedWallClockThreadFilterTest extends WallClockThreadFilterTest {
+    @Override
+    protected boolean isPlatformSupported() {
+        return Platform.isJ9();
+    }
 
     @Override
     protected String getProfilerCommand() {