RFR: 8266723: JFR periodic events are causing extra allocations

[Jaroslav Bachorík]

On Mon, May 17, 2021 at 2:13 PM Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>
>
>
> On Mon, May 17, 2021 at 2:11 PM Andrew Dinn <adinn at redhat.com> wrote:
>>
>>
>> On 17/05/2021 12:44, Jaroslav Bachorík wrote:
>> > JDK 11 has the full implementation of JVM logging and the
>> > 'should_log()' implementation is based on it.
>>
>> >> One small point, is it not excessive to always dive into native
>> >> coding for querying the log state? How hot is this coding? Ideally
>> >> logging should not cause overhead if logging is disabled. Would it not
>> >> be sufficient to cache the value in java?
>> >
>> > Good point. I will add caching.
>> That first statement makes me wonder about the need for this? Is the log
>> setting also cached in jdk11? If not then I wonder whether this is
>> really needed.
>>
>
> I agree, we should have the same solution in both releases.
>
> Wrt caching, logging can be switched off and on via jcmd, so caching is a trade off function wise. That's why I asked how hot the code is.

Anyway, caching does not seem feasible if we want to keep signatures
compatible with what we have in 9+ (and I am quite sure we want to do
that in order not to complicate any future backports) it should stay
`Logger.shouldLog(LogTag, LogLevel)` even though the underlying
implementation in JDK 8 knows nothing about any log tags and log
levels and defers to the `LogJFR` JVM argument value.

>From what I've seen the 'hottest' path would be the native method
sample where `Logger.shouldSample()` would be called (sampling
frequency * number of threads) times per second.

I would suggest not changing the code and keep the patch simple for
the sake of clarity and and address the potential perf improvements as
a followup (if I am able to get a measurable improvement with
caching).

Cheers,

-JB-

>
> Cheers, Thomas
>
>>
>> regards,
>>
>>
>> Andrew Dinn
>> -----------
>> Red Hat Distinguished Engineer
>> Red Hat UK Ltd
>> Registered in England and Wales under Company Registration No. 03798903
>> Directors: Michael Cunningham, Michael ("Mike") O'Neill
>>