On Wed, 4 Jun 2025 12:56:22 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: > > Fix build Some more cosmetics/nits. src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.cpp line 264: > 262: } > 263: timer_delete(*timer); > 264: tl->unset_cpu_timer(); Should this deletion be right in `unset_cpu_timer`? src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.cpp line 729: > 727: #else > 728: > 729: static bool _showed_warning = false; `_displayed_warning`? Actually, I think you can move this straight into `warn()` body. src/hotspot/share/jfr/periodic/sampling/jfrThreadSampling.cpp line 250: > 248: } > 249: > 250: Unnecessary? src/hotspot/share/jfr/support/jfrThreadLocal.cpp line 582: > 580: } > 581: > 582: bool JfrThreadLocal::acquire_cpu_time_jfr_enqueue_lock() { This sounds like `try_acquire_cpu_time_jfr_enqueue_lock`, emphasis on `try_`. It does not actually guarantee to lock. src/hotspot/share/jfr/support/jfrThreadLocal.cpp line 586: > 584: } > 585: > 586: bool JfrThreadLocal::try_acquire_cpu_time_jfr_dequeue_lock() { ...and this one is not `try_`, but the actual "blocking" acquire. ------------- PR Review: https://git.openjdk.org/jdk/pull/25302#pullrequestreview-2896958686 PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2126882197 PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2126868308 PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2126736956 PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2126831195 PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2126844713
