RFR: 8342818: Implement JEP 509: JFR CPU-Time Profiling [v5]

[Johannes Bechberger]

On Sun, 25 May 2025 16:17:12 GMT, Markus Grönlund <mgronlun at openjdk.org> wrote:

>> Johannes Bechberger has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix compilation
>
> src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.cpp line 373:
> 
>> 371:     }
>> 372:   }
>> 373:   assert(first_index == 0, "invariant");
> 
> How is this possible?

Because we have a safepoint before the thread goes into native (as far as I understand). I'll remove the code above, because it is therefore not needed.

> src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.cpp line 590:
> 
>> 588:   // so samples might be skipped and we have to compute the actual period
>> 589:   int64_t period = get_sampling_period() * (info->si_overrun + 1);
>> 590:   request._cpu_time_period = Ticks(period / 1000000000.0 * JfrTime::frequency()) - Ticks(0);
> 
> Are you treating JfrTime::frequency() as nanos here? JfrTime::frequency() can be in ticks, hence not a valid conversion.

Could you give me a hand with the conversion?

> src/hotspot/share/jfr/periodic/sampling/jfrThreadSampling.cpp line 375:
> 
>> 373:     }
>> 374:     queue.clear();
>> 375:     tl->release_cpu_time_jfr_queue_lock();
> 
> Is this releasing a different lock from the one acquired? "dequeue" lock vs "queue" lock?

These are not really different locks, as the lock has four different states. So it just changes the lock state from DEQUEUE to UNLOCKED.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2106637673
PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2106642499
PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2106639331