RFR: 8241678: Remove PerfData sampling via StatSampler
Albert Mingkun Yang
ayang at openjdk.org
Sat Apr 26 11:33:51 UTC 2025
On Fri, 25 Apr 2025 10:38:39 GMT, Casper Norrbin <cnorrbin at openjdk.org> wrote:
> Hi everyone,
>
> This change removes the legacy `PerfData` sampling mechanism implemented through the `StatSampler` — an always-on periodic task that runs every 50ms my default. The sampling feature was originally introduced to collect performance counters and timestamps, but has since seen very little use.
>
> For G1/ZGC, the only sampled value is a timestamp (`sun.os.hrt.ticks`). For Serial/Parallel, it also samples some heap space counters, but these are already updated after each GC cycle, making the sampling redundant. With sampling removed, the `PerfDataSamplingInterval` flag becomes obsoleted, as it no longer serves any purpose.
>
> The only thing relying on the sampled timestamps is `jstat`: running `jstat -t` prints an extra column with the time since VM start. To preserve this funcitonality, we can calculate the timestamps as an offset from the already existing `sun.rt.createVmBeginTime` instead.
There are still a few matches for "hrt.ticks"; don't know if they should be removed (in this PR or a followup).
src/hotspot/share/runtime/arguments.cpp line 538:
> 536: { "ZGenerational", JDK_Version::jdk(23), JDK_Version::jdk(24), JDK_Version::undefined() },
> 537: { "ZMarkStackSpaceLimit", JDK_Version::undefined(), JDK_Version::jdk(25), JDK_Version::undefined() },
> 538: { "PerfDataSamplingInterval", JDK_Version::undefined(), JDK_Version::jdk(25), JDK_Version::undefined() },
Since the CSR says it will be removed in 26, the final arg should reflect that, IMO.
src/hotspot/share/runtime/perfData.cpp line 419:
> 417: */
> 418: void PerfDataManager::assert_system_property(const char* name, const char* value, TRAPS) {
> 419: #ifdef ASSERT
The indentation seems odd. (I recall `#ifdef` itself should not indented.)
src/hotspot/share/runtime/perfData.cpp line 455:
> 453: assert(value != nullptr, "property name should be have a value: %s", name);
> 454: assert_system_property(name, value, CHECK);
> 455: if (value != nullptr) {
Why checking null again? Didn't we just asserted that 2 lines above?
src/hotspot/share/runtime/threads.cpp line 852:
> 850: #endif // INCLUDE_MANAGEMENT
> 851:
> 852: PerfDataManager::create_misc_perfdata();
Should this be guarded by `UsePerfData`?
-------------
PR Review: https://git.openjdk.org/jdk/pull/24872#pullrequestreview-2795935371
PR Review Comment: https://git.openjdk.org/jdk/pull/24872#discussion_r2061262368
PR Review Comment: https://git.openjdk.org/jdk/pull/24872#discussion_r2061263070
PR Review Comment: https://git.openjdk.org/jdk/pull/24872#discussion_r2061263372
PR Review Comment: https://git.openjdk.org/jdk/pull/24872#discussion_r2061264533
More information about the hotspot-gc-dev
mailing list