Code review

DataDog · zhengyu123 · Apr 9, 2026 · Apr 2, 2026 · Apr 3, 2026 · Apr 3, 2026
commit b035e63c70e54434c866ddd6fd9c1749481080c2
@@ -10,10 +10,6 @@ inline int makeFrame(ASGCT_CallFrame *frames, jint type, jmethodID id) {
   return 1;
 }
 
-inline int makeFrame(ASGCT_CallFrame *frames, jint type, uintptr_t id) {
-  return makeFrame(frames, type, (jmethodID)id);
-}
-
 inline int makeFrame(ASGCT_CallFrame *frames, jint type,
                             const char *id) {
   return makeFrame(frames, type, (jmethodID)id);

@@ -20,7 +20,7 @@ using StackWalkValidation::aligned;
 using StackWalkValidation::MAX_FRAME_SIZE;
 using StackWalkValidation::sameStack;
 
-bool isAddressInCode(const void *pc, bool include_stubs = true) {
+static bool isAddressInCode(const void *pc, bool include_stubs = true) {
   if (CodeHeap::contains(pc)) {
     return CodeHeap::findNMethod(pc) != NULL &&
            (include_stubs || !JitCodeCache::isCallStub(pc));
@@ -34,7 +34,7 @@ bool isAddressInCode(const void *pc, bool include_stubs = true) {
  *
  * This conversion is necessary because Datadog's implementation uses BCI_* values
  * (from ASGCT_CallFrameType) directly as event type identifiers, while upstream
- * StackWalker::walkVM() expects EventType enum values for its logic.
+ * HotspotSupport::walkVM() expects EventType enum values for its logic.
  *
  * BCI_* values are special frame types with negative values (except BCI_CPU=0)
  * that indicate non-standard frame information in call traces. EventType values
@@ -119,32 +119,6 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex
     }
 }
 
-__attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontext, ASGCT_CallFrame* frames, int max_depth, VMJavaFrameAnchor* anchor, EventType event_type, int lock_index, bool* truncated) {
-    uintptr_t sp = anchor->lastJavaSP();
-    if (sp == 0) {
-        return 0;
-    }
-
-    uintptr_t fp = anchor->lastJavaFP();
-    if (fp == 0) {
-        fp = sp;
-    }
-
-    const void* pc = anchor->lastJavaPC();
-    if (pc == NULL || !CodeHeap::contains(pc)) {
-        // lastJavaPC is NULL (thread not in Java→native transition) or points outside
-        // the tracked CodeHeap range (e.g. interpreter/stub code in a separately mmap'd
-        // region). Read the actual return address from the stack frame instead.
-        if (!aligned(sp)) {
-            return 0;
-        }
-        pc = ((const void**)sp)[-1];
-    }
-
-    StackWalkFeatures no_features{};
-    return walkVM(ucontext, frames, max_depth, no_features, event_type, pc, sp, fp, lock_index, truncated);
-}
-
 __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontext, ASGCT_CallFrame* frames, int max_depth,
                         StackWalkFeatures features, EventType event_type,
                         const void* pc, uintptr_t sp, uintptr_t fp, int lock_index, bool* truncated) {

@@ -19,9 +19,6 @@ class HotspotSupport {
     static int walkVM(void* ucontext, ASGCT_CallFrame* frames, int max_depth, 
                       StackWalkFeatures features, EventType event_type,
                       int lock_index, bool* truncated = nullptr);
-    static int walkVM(void* ucontext, ASGCT_CallFrame* frames, int max_depth,
-                      VMJavaFrameAnchor* anchor, EventType event_type, int lock_index,
-                      bool* truncated = nullptr);
 
     static int getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames,
                                  int max_depth, StackContext *java_ctx,

@@ -29,9 +29,10 @@ class JitCodeCache {
                                            const void *address, jint length);
 
     static inline bool isCallStub(const void *address) {
-        return _call_stub_end.load(std::memory_order_acquire) != nullptr &&
-               address >= _call_stub_begin.load(std::memory_order_relaxed) &&
-               address < _call_stub_end.load(std::memory_order_relaxed);
+        const void* stub_end = _call_stub_end.load(std::memory_order_acquire);
+        return stub_end != nullptr &&
+            address >= _call_stub_begin.load(std::memory_order_relaxed) &&
+            address < stub_end;    
     }
 
     static CodeBlob* findRuntimeStub(const void *address);

@@ -120,8 +120,6 @@ class J9Support {
     return asgct;
   }
 
-  static int walkJavaStack(StackWalkRequest& request);
-
   static bool initialize(jvmtiEnv *jvmti, const void *j9thread_self);
 
   static JNIEnv *GetJ9vmThread(jthread thread) {

@@ -47,6 +47,6 @@ int JVMSupport::asyncGetCallTrace(ASGCT_CallFrame *frames, int max_depth, void*
         return 0;
     }
 
-    Profiler::instance()->incFailure(trace.num_frames);
+    Profiler::instance()->incFailure(-trace.num_frames);
     return makeFrame(frames, BCI_ERROR, err_string);
 }
@@ -133,8 +133,6 @@ class alignas(alignof(SpinLock)) Profiler {
   u32 getLockIndex(int tid);
   int getNativeTrace(void *ucontext, ASGCT_CallFrame *frames, int event_type,
                      int tid, StackContext *java_ctx, bool *truncated, int lock_index);
-  int getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, int max_depth,
-                        StackContext *java_ctx, bool *truncated);
   void updateThreadName(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread,
                         bool self = false);
   void updateJavaThreadNames();

@@ -71,7 +71,7 @@ namespace StackWalkValidation {
     }
 }
 
-typedef struct _StackWalkReuqest {
+typedef struct _StackWalkRequest {
     jint event_type;
     u32 lock_index;
     void* ucontext;