On Sat, 31 May 2025 10:22:47 GMT, Markus Grönlund <mgron...@openjdk.org> wrote:
>> Johannes Bechberger has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove debug printf
>
> src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.cpp line 363:
>
>> 361: }
>> 362:
>> 363: void JfrCPUTimeThreadSampler::stackwalk_thread_in_native(JavaThread*
>> thread) {
>
> I still don't understand what the purpose is for this routine.
>
> I understand that this is to handle the situation where a thread is in state
> _thread_in_native, and cannot be handled immediately. But what problem are
> you trying to solve?
>
> It seems there is some convoluted logic to only locate and process those
> requests that correspond to samples where the thread would be in native? But
> what purpose does that serve?
>
> In JFR Cooperative Sampling, we allow the sampler thread to drain the entire
> sample request queue for a thread when it is found to be running in state
> _thread_in_native, on the premise that we cannot know or guarantee if or when
> a thread will return to the VM.
>
> This seems to be some kind of semi-solution that does not solve anything:
>
> 1. If you don't care about samples being processed and delivered swiftly, you
> don't even need the sampler thread to process native frames - do nothing and
> let the thread process everything on return from native (those native
> requests are still valid (the ljf is still valid).
> 2. If you want swift delivery of samples, you drain the entire queue, not
> just some native samples.
Secondly, if you only want to process native requests that are equal to or
correspond to a specific native site, you should only need to call
stacktrace.record() inner for one of the sample requests, not all, and reuse
the sid for all the requests.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2117688216
