On Sun, 25 May 2025 16:17:12 GMT, Markus Grönlund <mgron...@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
