Skip to content

Commit 93ab936

Browse files
rkennkeclaude
andauthored
feat(profiler): add jvmtistacks option to use JVMTI/JFR stack-walker (PROF-14350) (#504)
Adds a new jvmtistacks profiler option that, when enabled, delegates CPU and wall-clock stack-trace collection to HotSpot's JFR RequestStackTrace JVMTI extension (available in JDK 27+). This allows the profiler to leverage the JVM's own safepoint-based stack walker instead of ASGCT, which improves reliability in cases where ASGCT cannot walk the stack. Engine selection: - CPU: prefers CTimerJvmti (per-thread timers) → ITimerJvmti (setitimer) → falls back to ASGCT engines with a warning if neither is available - Wall-clock: uses WallClockJvmti when the extension is present Signal safety and correctness fixes included: - Guard against null fn pointer in VM::requestStackTrace() - Restore errno on early return in WallClockJvmti::signalHandler - Save/restore SIGPROF handler in ITimerJvmti::check() probe - Align recordEventDelegated() with recordEvent() semantics (break not return) - Remove no-op initializeRequestStackTrace() retry in Profiler::start() - Treat per-thread CTimer registration failures as benign during start() (threads may exit between listThreads() and registerThread()) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ea0a097 commit 93ab936

17 files changed

Lines changed: 617 additions & 22 deletions

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,21 @@ Error Arguments::parse(const char *args) {
364364
_remote_symbolication = true;
365365
}
366366

367+
CASE("jvmtistacks")
368+
if (value != NULL) {
369+
switch (value[0]) {
370+
case 'y': // yes
371+
case 't': // true
372+
case '1': // 1
373+
_jvmtistacks = true;
374+
break;
375+
default:
376+
_jvmtistacks = false;
377+
}
378+
} else {
379+
_jvmtistacks = true;
380+
}
381+
367382
CASE("wallsampler")
368383
if (value != NULL) {
369384
switch (value[0]) {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ class Arguments {
192192
bool _lightweight;
193193
bool _enable_method_cleanup;
194194
bool _remote_symbolication; // Enable remote symbolication for native frames
195+
bool _jvmtistacks; // Delegate CPU/wall stack walks to HotSpot JFR RequestStackTrace extension
195196

196197
Arguments(bool persistent = false)
197198
: _buf(NULL),
@@ -227,7 +228,8 @@ class Arguments {
227228
_context_attributes({}),
228229
_lightweight(false),
229230
_enable_method_cleanup(true),
230-
_remote_symbolication(false) {}
231+
_remote_symbolication(false),
232+
_jvmtistacks(false) {}
231233

232234
~Arguments();
233235

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,17 @@
9898
X(WALKVM_CONT_ENTRY_NULL, "walkvm_cont_entry_null") \
9999
X(NATIVE_LIBS_DROPPED, "native_libs_dropped") \
100100
X(SIGACTION_PATCHED_LIBS, "sigaction_patched_libs") \
101-
X(SIGACTION_INTERCEPTED, "sigaction_intercepted") \
102-
X(CTIMER_SIGNAL_OWN, "ctimer_signal_own") \
103-
X(CTIMER_SIGNAL_FOREIGN, "ctimer_signal_foreign") \
104-
X(WALLCLOCK_SIGNAL_OWN, "wallclock_signal_own") \
105-
X(WALLCLOCK_SIGNAL_FOREIGN, "wallclock_signal_foreign")
101+
X(SIGACTION_INTERCEPTED, "sigaction_intercepted") \
102+
X(CTIMER_SIGNAL_OWN, "ctimer_signal_own") \
103+
X(CTIMER_SIGNAL_FOREIGN, "ctimer_signal_foreign") \
104+
X(WALLCLOCK_SIGNAL_OWN, "wallclock_signal_own") \
105+
X(WALLCLOCK_SIGNAL_FOREIGN, "wallclock_signal_foreign") \
106+
X(JVMTI_STACKS_INIT_OK, "jvmti_stacks_init_ok") \
107+
X(JVMTI_STACKS_INIT_FAILED, "jvmti_stacks_init_failed") \
108+
X(JVMTI_STACKS_REQUESTED, "jvmti_stacks_requested") \
109+
X(JVMTI_STACKS_FAILED_WRONG_PHASE, "jvmti_stacks_failed_wrong_phase") \
110+
X(JVMTI_STACKS_FAILED_OTHER, "jvmti_stacks_failed_other") \
111+
X(JVMTI_STACKS_DROPPED_LOCK, "jvmti_stacks_dropped_lock")
106112
#define X_ENUM(a, b) a,
107113
typedef enum CounterId : int {
108114
DD_COUNTER_TABLE(X_ENUM) DD_NUM_COUNTERS

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
#include <signal.h>
2626

2727
class CTimer : public Engine {
28-
private:
28+
protected:
2929
// This is accessed from signal handlers, so must be async-signal-safe
3030
static bool _enabled;
3131
static long _interval;
@@ -38,6 +38,7 @@ class CTimer : public Engine {
3838
int registerThread(int tid);
3939
void unregisterThread(int tid);
4040

41+
private:
4142
// cppcheck-suppress unusedPrivateFunction
4243
static void signalHandler(int signo, siginfo_t *siginfo, void *ucontext);
4344

@@ -60,6 +61,24 @@ class CTimer : public Engine {
6061
static int getSignal() { return _signal; }
6162
};
6263

64+
// A CPU-time engine that reuses CTimer's per-thread timer_create / SIGPROF
65+
// dispatch, but instead of walking the stack in the signal handler delegates
66+
// the walk to HotSpot's JFR RequestStackTrace JVMTI extension. The sampled
67+
// event is emitted on our side with only a correlation ID; the JVM writes
68+
// the stack trace (and its own JFR stack-trace id) into the concurrent JFR
69+
// recording as jdk.StackTraceRequest. See VM::canRequestStackTrace().
70+
class CTimerJvmti : public CTimer {
71+
private:
72+
// cppcheck-suppress unusedPrivateFunction
73+
static void signalHandler(int signo, siginfo_t *siginfo, void *ucontext);
74+
75+
public:
76+
const char *name() { return "CTimerJvmti"; }
77+
78+
Error check(Arguments &args);
79+
Error start(Arguments &args);
80+
};
81+
6382
#else
6483

6584
class CTimer : public Engine {
@@ -75,6 +94,19 @@ class CTimer : public Engine {
7594
static bool supported() { return false; }
7695
};
7796

97+
class CTimerJvmti : public Engine {
98+
public:
99+
const char *name() { return "CTimerJvmti"; }
100+
101+
Error check(Arguments &args) {
102+
return Error("CTimerJvmti is not supported on this platform");
103+
}
104+
105+
Error start(Arguments &args) {
106+
return Error("CTimerJvmti is not supported on this platform");
107+
}
108+
};
109+
78110
#endif // __linux__
79111

80112
#endif // _CTIMER_H

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

Lines changed: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,15 +163,13 @@ Error CTimer::start(Arguments &args) {
163163

164164
OS::installSignalHandler(_signal, signalHandler);
165165

166-
// Register all existing threads
167-
Error result = Error::OK;
166+
// Register all existing threads. Individual failures are benign — a thread
167+
// may exit between listThreads() and registerThread(), and new threads
168+
// will register themselves on creation. check() already validated that the
169+
// timer mechanism works on this system.
168170
ThreadList *thread_list = OS::listThreads();
169-
while (thread_list->hasNext()) {
170-
int tid = thread_list->next();
171-
int err = registerThread(tid);
172-
if (err != 0) {
173-
result = Error("Failed to register thread");
174-
}
171+
while (thread_list->hasNext()) {
172+
registerThread(thread_list->next());
175173
}
176174
delete thread_list;
177175

@@ -184,6 +182,71 @@ void CTimer::stop() {
184182
}
185183
}
186184

185+
Error CTimerJvmti::check(Arguments &args) {
186+
if (!VM::canRequestStackTrace()) {
187+
return Error("HotSpot RequestStackTrace JVMTI extension not available");
188+
}
189+
return CTimer::check(args);
190+
}
191+
192+
Error CTimerJvmti::start(Arguments &args) {
193+
if (!VM::canRequestStackTrace()) {
194+
return Error("HotSpot RequestStackTrace JVMTI extension not available");
195+
}
196+
Error result = CTimer::start(args);
197+
if (result) return result;
198+
// Override the signal handler installed by CTimer::start with our own,
199+
// which delegates stack walking to the HotSpot JFR extension.
200+
OS::installSignalHandler(_signal, CTimerJvmti::signalHandler);
201+
return Error::OK;
202+
}
203+
204+
void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
205+
if (!OS::shouldProcessSignal(siginfo, SI_TIMER, SignalCookie::cpu())) {
206+
Counters::increment(CTIMER_SIGNAL_FOREIGN);
207+
OS::forwardForeignSignal(signo, siginfo, ucontext);
208+
return;
209+
}
210+
Counters::increment(CTIMER_SIGNAL_OWN);
211+
212+
CriticalSection cs;
213+
if (!cs.entered()) {
214+
return;
215+
}
216+
int saved_errno = errno;
217+
if (!__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE)) {
218+
errno = saved_errno;
219+
return;
220+
}
221+
int tid = 0;
222+
ProfiledThread *current = ProfiledThread::currentSignalSafe();
223+
assert(current == nullptr || !current->isDeepCrashHandler());
224+
if (current != nullptr && JVMThread::isInitialized() && JVMThread::current() == nullptr
225+
&& current->inInitWindow()) {
226+
current->tickInitWindow();
227+
errno = saved_errno;
228+
return;
229+
}
230+
if (current != NULL) {
231+
current->noteCPUSample(Profiler::instance()->recordingEpoch());
232+
tid = current->tid();
233+
} else {
234+
tid = OS::threadId();
235+
}
236+
Shims::instance().setSighandlerTid(tid);
237+
238+
ExecutionEvent event;
239+
event._execution_mode = getThreadExecutionMode();
240+
// Opted into JVMTI delegation; drop the sample if the JVM rejects the
241+
// request (WRONG_PHASE if JFR is not recording, NOT_AVAILABLE if
242+
// jdk.StackTraceRequest is disabled). recordSampleDelegated() bumps the
243+
// failure counters; there is no fallback to ASGCT in this engine.
244+
Profiler::instance()->recordSampleDelegated(ucontext, _interval, tid,
245+
BCI_CPU, &event);
246+
Shims::instance().setSighandlerTid(-1);
247+
errno = saved_errno;
248+
}
249+
187250
void CTimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
188251
// Reject signals that did not originate from our timer_create timers.
189252
// This guards against Go's process-wide setitimer(ITIMER_PROF) and other

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

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,6 +1520,7 @@ void Recording::writeEventSizePrefix(Buffer *buf, int start) {
15201520
}
15211521

15221522
void Recording::recordExecutionSample(Buffer *buf, int tid, u64 call_trace_id,
1523+
u64 correlation_id,
15231524
ExecutionEvent *event) {
15241525
int start = buf->skip(1);
15251526
buf->putVar64(T_EXECUTION_SAMPLE);
@@ -1529,12 +1530,14 @@ void Recording::recordExecutionSample(Buffer *buf, int tid, u64 call_trace_id,
15291530
buf->put8(static_cast<int>(event->_thread_state));
15301531
buf->put8(static_cast<int>(event->_execution_mode));
15311532
buf->putVar64(event->_weight);
1533+
buf->putVar64(correlation_id);
15321534
writeCurrentContext(buf);
15331535
writeEventSizePrefix(buf, start);
15341536
flushIfNeeded(buf);
15351537
}
15361538

15371539
void Recording::recordMethodSample(Buffer *buf, int tid, u64 call_trace_id,
1540+
u64 correlation_id,
15381541
ExecutionEvent *event) {
15391542
int start = buf->skip(1);
15401543
buf->putVar64(T_METHOD_SAMPLE);
@@ -1544,6 +1547,7 @@ void Recording::recordMethodSample(Buffer *buf, int tid, u64 call_trace_id,
15441547
buf->put8(static_cast<int>(event->_thread_state));
15451548
buf->put8(static_cast<int>(event->_execution_mode));
15461549
buf->putVar64(event->_weight);
1550+
buf->putVar64(correlation_id);
15471551
writeCurrentContext(buf);
15481552
writeEventSizePrefix(buf, start);
15491553
flushIfNeeded(buf);
@@ -1848,11 +1852,11 @@ void FlightRecorder::recordEvent(int lock_index, int tid, u64 call_trace_id,
18481852
RecordingBuffer *buf = rec->buffer(lock_index);
18491853
switch (event_type) {
18501854
case BCI_CPU:
1851-
rec->recordExecutionSample(buf, tid, call_trace_id,
1855+
rec->recordExecutionSample(buf, tid, call_trace_id, 0,
18521856
(ExecutionEvent *)event);
18531857
break;
18541858
case BCI_WALL:
1855-
rec->recordMethodSample(buf, tid, call_trace_id,
1859+
rec->recordMethodSample(buf, tid, call_trace_id, 0,
18561860
(ExecutionEvent *)event);
18571861
break;
18581862
case BCI_ALLOC:
@@ -1878,6 +1882,32 @@ void FlightRecorder::recordEvent(int lock_index, int tid, u64 call_trace_id,
18781882
}
18791883
}
18801884

1885+
void FlightRecorder::recordEventDelegated(int lock_index, int tid,
1886+
u64 correlation_id, int event_type,
1887+
Event *event) {
1888+
OptionalSharedLockGuard locker(&_rec_lock);
1889+
if (locker.ownsLock()) {
1890+
Recording* rec = _rec;
1891+
if (rec != nullptr) {
1892+
RecordingBuffer *buf = rec->buffer(lock_index);
1893+
switch (event_type) {
1894+
case BCI_CPU:
1895+
rec->recordExecutionSample(buf, tid, 0, correlation_id,
1896+
(ExecutionEvent *)event);
1897+
break;
1898+
case BCI_WALL:
1899+
rec->recordMethodSample(buf, tid, 0, correlation_id,
1900+
(ExecutionEvent *)event);
1901+
break;
1902+
default:
1903+
// Delegation is only wired for CPU/wall samples in v1.
1904+
break;
1905+
}
1906+
rec->addThread(lock_index, tid);
1907+
}
1908+
}
1909+
}
1910+
18811911
void FlightRecorder::recordLog(LogLevel level, const char *message,
18821912
size_t len) {
18831913
OptionalSharedLockGuard locker(&_rec_lock);

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,9 +276,9 @@ class Recording {
276276
void writeCurrentContext(Buffer *buf);
277277

278278
void recordExecutionSample(Buffer *buf, int tid, u64 call_trace_id,
279-
ExecutionEvent *event);
279+
u64 correlation_id, ExecutionEvent *event);
280280
void recordMethodSample(Buffer *buf, int tid, u64 call_trace_id,
281-
ExecutionEvent *event);
281+
u64 correlation_id, ExecutionEvent *event);
282282
void recordWallClockEpoch(Buffer *buf, WallClockEpochEvent *event);
283283
void recordTraceRoot(Buffer *buf, int tid, TraceRootEvent *event);
284284
void recordQueueTime(Buffer *buf, int tid, QueueTimeEvent *event);
@@ -356,6 +356,13 @@ class FlightRecorder {
356356
void recordEvent(int lock_index, int tid, u64 call_trace_id, int event_type,
357357
Event *event);
358358

359+
// Emit a BCI_CPU / BCI_WALL sample with no stack-trace attached to our
360+
// recording. `correlation_id` is the same jlong passed to the HotSpot
361+
// RequestStackTrace extension so downstream tooling can join our event with
362+
// the JVM-emitted jdk.StackTraceRequest.
363+
void recordEventDelegated(int lock_index, int tid, u64 correlation_id,
364+
int event_type, Event *event);
365+
359366
void recordLog(LogLevel level, const char *message, size_t len);
360367

361368
void recordDatadogSetting(int lock_index, int length, const char *name,

ddprof-lib/src/main/cpp/hotspot/vmStructs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ typedef void* address;
300300
field(_flag_type_offset, offset, MATCH_SYMBOLS("_type", "type")) \
301301
type_end() \
302302
type_begin(VMOop, MATCH_SYMBOLS("oopDesc")) \
303-
field(_oop_klass_offset, offset, MATCH_SYMBOLS("_metadata._klass")) \
303+
field(_oop_klass_offset, offset, MATCH_SYMBOLS("_metadata._klass", "_compressed_klass")) \
304304
type_end() \
305305
type_begin(VMUniverse, MATCH_SYMBOLS("Universe", "CompressedKlassPointers")) \
306306
field(_narrow_klass_base_addr, address, MATCH_SYMBOLS("_narrow_klass._base", "_base")) \

0 commit comments

Comments
 (0)