On Fri, 2 Oct 2026 09:26:54 GMT, Roman Kennke <[email protected]> wrote:

>> Implements the native profiler hook for unbiased stack-traces, as described 
>> in [JEP draft: Native Profiler Hook for Unbiased Stack Traces 
>> (Experimental)](https://openjdk.org/jeps/8380294).
>> 
>> New JVMTI extension function: `RequestStackTrace(jvmtiEnv*, jthread*, void* 
>> ucontext, jlong user_data)` - async-signal-safe request for a stack trace. 
>> For the current thread, `ucontext` may be `nullptr` or the signal-handler 
>> `ucontext`. For another thread, the thread must be suspended for the 
>> duration of the call and `ucontext` must be captured during that suspension; 
>> otherwise the VM may crash. Returns `JVMTI_ERROR_NONE`, 
>> `JVMTI_ERROR_INVALID_THREAD`, or `JVMTI_ERROR_NOT_AVAILABLE`. Requests may 
>> be silently dropped (queue full, wrong thread state, rate-limited).
>> 
>> New experimental JFR event `StackTraceRequest` with fields `stackTrace`, 
>> `eventThread`, `userData`, `failed`, `biased`. Enabled in `profile.jfc`, not 
>> in `default.jfc` (the rate is controlled by the external agent, so 
>> default-on would expose users to unbounded overhead).
>> 
>> The machinery gets enabled/disabled whenever the event `StackTraceRequest` 
>> gets enabled/disabled.
>> 
>> Refactoring: The stack-walker request machinery from JEP 509 (CPU-time 
>> profiler) is generalized into a callback-based API 
>> (`JfrStackWalkerCallback`) used by both the CPU-time profiler and the new 
>> JVMTI hook. Shared pieces: the per-thread signal-safe request queue, the 
>> enqueue/dequeue lock state machine, and the native stack-walker thread that 
>> processes requests for in-native threads. Two callback implementations 
>> remain: one for CPUTimeSample/empty/lost events, one for StackTraceRequest.
>> 
>> Non-goals (per the JEP): Native frames are not reported; this is a follow-up.
>> 
>> Platform support: Linux and MacOS, or more generally POSIX. Windows support 
>> can be a follow-up, if ucontext is interpreted as WPA context.
>> 
>> Testing:
>> 
>> Several tests have been added that verify the new functionality.
>> 
>>  - [x] jdk_jfr
>>  - [x] serviceability/jvmti
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Roman Kennke has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Improve TestRequestStackTraceForeignThread to be more reliable under load

src/hotspot/share/jfr/periodic/sampling/jfrStackWalker.cpp line 608:

> 606: }
> 607: 
> 608: static bool build_from_ljf(JfrStackWalkRequest& request,

This function is almost identical to jfrSampleRequest.cpp/build_from_ljf. 
Should we combine them? it's hard to sell that we have two parallel lines in 
JFR.

src/hotspot/share/jfr/periodic/sampling/jfrStackWalker.cpp line 939:

> 937: 
> 938:   in_continuation = is_in_continuation(top_frame, jt);
> 939:   return true;

There's a potential risk for 'top_frame'. I know it's not this PR introduces 
it. just call it out. 

We actually generate an empty top_frame at line 1033. Currently, it's defined. 
Everything is null because all platforms define its constructor frame::frame().

I think it's possible compute_top_frame() returns true but never get assigned 
by any valid frame. eg. stream.is_done() == true so we skip the loop entirely. 

1. skip to report failure because it returns true.  
2. JfrStackWalkerVframeStream will walk the stack starting with this dummy 
frame. I am not sure it's 100% safe.

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 604:

> 602:   assert_frames_in_continuation_are_safe(_thread);
> 603:   JFR_ONLY(JfrStackWalker::check_and_process_requests(_thread));
> 604:   JFR_ONLY(Jfr::check_and_process_sample_request(_thread));

Can we consider merge JfrStackWalker::check_and_process_requests(jt) into 
Jfr::check_and_process_sample_request(jt)?

In the prior PR, JVMTI RequestStackTrace was an independent feature. now it 
depends on JFR in runtime. We can't use it without --enable-jfr. I would say 
that we just embrace that. 

1. If we only update `Jfr::check_and_process_sample_request(jt)`, we will 
shrink the patch size. We don't need to update runtime functions. The effect is 
same. You can check JFRStackWalker first. We can also avoid including details 
like jfrStackWalker.ineline.hpp. 

2. One utility function is also good for maintenance. We will never forget they 
appear in pairs.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/31059#discussion_r4332161808
PR Review Comment: https://git.openjdk.org/jdk/pull/31059#discussion_r4333323490
PR Review Comment: https://git.openjdk.org/jdk/pull/31059#discussion_r4333649654

Reply via email to