On Fri, 30 May 2025 10:17:18 GMT, Johannes Bechberger <jbechber...@openjdk.org> wrote:
>> This is the code for the [JEP 509: CPU Time based profiling for >> JFR](https://openjdk.org/jeps/509). >> >> Currently tested using [this test >> suite](https://github.com/parttimenerd/basic-profiler-tests). This runs >> profiles the [Renaissance](https://renaissance.dev/) benchmark with >> - ... different heap sizes >> - ... different GCs >> - ... different samplers (the standard JFR and the new CPU Time Sampler and >> both) >> - ... different JFR recording durations >> - ... different chunk-sizes > > 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2117682624
