RFR: 8358619: Fix interval recomputation in CPU Time Profiler

[Andrei Pangin]

On Thu, 12 Jun 2025 09:27:30 GMT, Johannes Bechberger <jbechberger at openjdk.org> wrote:

> Fixes the recomputation issue by passing either the rate or the period directly to the sampler (instead of just the rate value with the period value converted into a rate). This prevents issues when the number of cores changes during the JFR recording.

src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformEventType.java line 205:

> 203:     public void setCPUThrottle(TimespanRateOrPeriod rate) {
> 204:         if (isCPUTimeMethodSampling) {
> 205:             System.out.println("Setting CPU throttle for " + getName() + " to " + rate);

Please remove debug printout here and in other places.

src/jdk.jfr/share/classes/jdk/jfr/internal/util/TimespanRateOrPeriod.java line 59:

> 57:     }
> 58: 
> 59:     public static TimespanRateOrPeriod max(TimespanRateOrPeriod a, TimespanRateOrPeriod b) {

`max` is not the best name for it, since it can be `min` for period or `max` for rate.
The idea of the method is to select a value with the highest resolution, right? Let's reflect this in the name and/or code comment.

src/jdk.jfr/share/classes/jdk/jfr/internal/util/TimespanRateOrPeriod.java line 67:

> 65:         }
> 66:         if (a.isRate) {
> 67:             double bRate = Runtime.getRuntime().availableProcessors() / b.periodNanos() * 1_000_000_000.0;

Integer division will round it to zero. Correct expression would be
`Runtime.getRuntime().availableProcessors() * (1_000_000_000.0 / b.periodNanos())`

src/jdk.jfr/share/classes/jdk/jfr/internal/util/TimespanRateOrPeriod.java line 70:

> 68:             return new TimespanRateOrPeriod(Math.max(a.rate(), bRate), 0, true);
> 69:         }
> 70:         return max(b, a); // swap to use the same logic

Please don't use recursion for trivial cases.

src/jdk.jfr/share/classes/jdk/jfr/internal/util/TimespanRateOrPeriod.java line 95:

> 93:         }
> 94:         // fallback to days if no smaller unit is found
> 95:         return String.format("%d/%s", (long)(rate / TimespanUnit.DAYS.nanos * 1_000_000_000.0), TimespanUnit.DAYS.text);

Do we really need this complication? Where is this `toString()` used?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25775#discussion_r2178359999
PR Review Comment: https://git.openjdk.org/jdk/pull/25775#discussion_r2178405607
PR Review Comment: https://git.openjdk.org/jdk/pull/25775#discussion_r2178374195
PR Review Comment: https://git.openjdk.org/jdk/pull/25775#discussion_r2178375571
PR Review Comment: https://git.openjdk.org/jdk/pull/25775#discussion_r2178399206