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