RFR: 8257621: JFR StringPool misses cached items across consecutive recordings

[Erik Gahlin]

On Fri, 4 Dec 2020 17:10:58 GMT, Jie Kang <jkang at openjdk.org> wrote:

>> Great analysis, makes sense. 
>> 
>> The test paths I see in the test system is about 80 characters.
>
> The problem scenario in more detail is:
> 
> StringPool has a Java side cache for Strings that meet the caching requirements. jfrStringPool.hpp/cpp is the native accompaniment that is called to write strings into the constant pool if a string is submitted to the Java layer cache that does not already exist, or is submitted for a *new* epoch.
> 
> Start recording A, at epoch 0. Emit 2 events with a String "s". This does nto exist in the Java cache, so is cached and pushed into the native layer to emit as a constant pool. Stop recording A. The event will have message=1, and constant pool will have java.lang.String entry:  `1="s"`.
> 
> Start recording B, still at epoch 0. Emit 2 events with a String "s". The Java layer cache still contains "s" for epoch 0. No request is made to push the string to the native layer to emit as a constant pool. Stop recording B. The event will have message=1, but there will be no constant pool entry for `1`.
> 
> My fix in more detail is:
> 
> Reset the Java layer cache across the recording boundary, specifically when a recording is stopped and there are no other running recordings. I was also thinking about rotating the epoch at this boundary, but I'm not sure what impacts it would have with the other jfr sub-systems.
> 
> I have also tested this fix in the simple "concurrent recording" case where I start recording A, start recording B, stop recording A and stop recording B.

I'm not sure either, Markus knows the epoch system the best, but I wouldn't change it.

We like to clear the cache (to reduce heap usage) when a physical recording ends, so I think your fix is fine for that reason alone. Possibly there should be a call to reset in PlatformRecorder::destroy() as well. It is however only called in the shutdown hook, so in practise it will probably not matter.

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

PR: https://git.openjdk.java.net/jdk/pull/1576