On Mon, 2 Jun 2025 11:32:27 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 two > additional commits since the last revision: > > - Tiny fixes > - Minor changes src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.cpp line 30: > 28: #include "runtime/orderAccess.hpp" > 29: #include "utilities/ticks.hpp" > 30: #include "jfr/periodic/sampling/jfrCPUTimeThreadSampler.hpp" Include order? src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.cpp line 60: > 58: assert(raw_thread->is_Java_thread(), "invariant"); > 59: JavaThread* jt; > 60: if ((jt = JavaThread::cast(raw_thread))->is_exiting()) { I see no point to be extra-smart with inline assignments here: Suggestion: JavaThread* jt = JavaThread::cast(raw_thread); if (jt->is_exiting()) { src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.cpp line 115: > 113: JfrCPUTimeSampleRequest* new_data = > JfrCHeapObj::new_array<JfrCPUTimeSampleRequest>(capacity); > 114: JfrCHeapObj::free(_data, _capacity * sizeof(JfrCPUTimeSampleRequest)); > 115: _data = new_data; A bit of peak memory consumption improvement: don't have two things live at once. Plus, give the native allocator a chance to reuse the same location. Suggestion: JfrCHeapObj::free(_data, _capacity * sizeof(JfrCPUTimeSampleRequest)); _data = JfrCHeapObj::new_array<JfrCPUTimeSampleRequest>(capacity); ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2120895107 PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2120897472 PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2120909443