RFR: 8277919: OldObjectSample event causing bloat in the class constant pool in JFR recording [v3]

[Markus Grönlund]

On Sun, 12 Dec 2021 07:33:04 GMT, Jaroslav Bachorik <jbachorik at openjdk.org> wrote:

>> Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   update assertion
>
> src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp line 205:
> 
>> 203: static bool stack_trace_precondition(const ObjectSample* sample) {
>> 204:   assert(sample != NULL, "invariant");
>> 205:   return sample->has_stack_trace_id() && !sample->is_dead() && !sample->stacktrace().valid();
> 
> I am curious - why is `!sample->stacktrace().valid()` required here?
> Is it because of the change from `writer.copy()` to `writer.move()`?

Hi Jaroslav, so this is another optimization to reduce the size:

The previous writer.copy() usage was because there needed to be a mechanism to ensure that the newest sample candidates also had their stacktraces serialized in the current chunk. Object samples are emitted (as part of recording dump/stop) before on_rotation(). The old object events would be emitted, but their stacktraces would be serialized in the next on_rotation() call. These "new candidate" stacktraces are not really needed in every chunk but only those that emit object samples. A way to reduce the number of stacktraces written in every chunk is to let the ObjectSampleCheckpoint::write() first create the needed stacktrace blobs right there before serializing them. Then, it will only lay down the blobs for the needed candidates, and the automatic serialization of object sample stacktraces (via writer.copy() which installs the data into the checkpoint queue) can be avoided (turned into writer.move()). With this change, the upcoming on_rotation() will have to navigate state correctl
 y since some of these new candidates already have stacktrace blobs associated (if there was a previous object sample emit call), and the system would assert. Hence the need to check if a stacktrace blob has already been created via the .valid() call.

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

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