RFR: 8342818: Implement CPU Time Profiling for JFR

[Erik Gahlin]

On Tue, 10 Sep 2024 13:48:44 GMT, Johannes Bechberger <jbechberger at openjdk.org> wrote:

> > How is throttling handled? We don't want buffers to be filled up with sample events?
> 
> By setting the internal queue size. The user can throttle by setting the interval, any other option would lead to bias.

Where can the queue size be set and how does a user know what is a reasonable value?

> Just to clarify, do you mean that you're against using "rate" as a setting name?

Name only, implementation as we discussed and I believe you have implemented.

>> src/jdk.jfr/share/classes/jdk/jfr/internal/util/ParsedCPUThrottle.java line 33:
>> 
>>> 31:  * A rate or fixed period, see {@link jdk.jfr.internal.Rate}
>>> 32:  */
>>> 33: public sealed interface ParsedCPUThrottle {
>> 
>> This seems complicated. Why isn't possible to keep the Rate class as is (no renaming) and then only create a TimespanRate class and do something like this:
>> 
>>     public record TimespanRate(double rate) {
>>     
>>         public static TimespanRate of(String text) {
>>            Rate r = Rate.of(text);
>>            if (r != null) {
>>               double rate = r.amount() * ...
>>               return new TimespanRate(rate)
>>            } else {
>>               long v = ValueParser.parseTimespanWithInfinity(text);
>>               double rate = v * ...
>>               return new TimespanRate(rate);
>>           }
>>       }
>>     
>>       public boolean isHigher(TimespanRate that) {
>>           return rate() > that.rate();
>>       }
>>     }
>
> I can try to implement. I thought it would be easier to comprehend this way.

Since you need custom logic in CPUThrottleSetting, it might be easier to just have two distinct types and skip the common interface?

I haven't looked at it closely, it's just my impression. Maybe there should be a ThrottlePeriod(double period, boolean autoAdapt) with a factory method where it first checks if it can be created from Rate.of(text).

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

PR Comment: https://git.openjdk.org/jdk/pull/20752#issuecomment-2365155428
PR Comment: https://git.openjdk.org/jdk/pull/20752#issuecomment-2416305503
PR Review Comment: https://git.openjdk.org/jdk/pull/20752#discussion_r1807679939