RFR: 8258431: Provide a JFR event with live set size estimate [v12]

[Stefan Johansson]

On Tue, 16 Mar 2021 09:37:08 GMT, Jaroslav Bachorik <jbachorik at openjdk.org> wrote:

>>> @kstefanj
>>> 
>>> > Then the heap expands and your application get to a steady state that doesn't require any more marking cycles.
>>> 
>>> Is there a way to get the liveness info when the heap expands? If not that would mean we had no way to figure out the new live set size and would assume, conservatively, the last known value.
>>> 
>>> As I mentioned in the PR description the live size value will be a 'best effort estimate' depending on what can each particular GC provide.
>> 
>> Sure, and this is fair, my concern is just that this 'best effort estimate' for G1 will often be worse than just using `used()`. This is not only a problem for when the heap expands, that was just an example, the live value will become more and more stale the longer an application run without triggering a new concurrent cycle. 
>> 
>> For the liveness value to be useful it would have to be updated at each GC, and we need to investigate further to see how we can do that in a "cheap" way. I would prefer if G1 did just return `used()` in `live()` as a start and we can create a follow-up task to investigate how to best add a better estimate. Do you see any problem with this?
>
>> For the liveness value to be useful it would have to be updated at each GC, and we need to investigate further to see how we can do that in a "cheap" way. I would prefer if G1 did just return used() in live() as a start and we can create a follow-up task to investigate how to best add a better estimate. Do you see any problem with this?
> 
> Actually yes - this event was meant to be a cheap way to see whether the *known* live size is growing (has an upwards trend) which would using the `used()` value make more difficult and unreliable. 
> 
> I would prefer keeping the implementation to return the lower bound of the live size as is the case for other GCs as well. May add explanatory comments to the code and the event definition to make this clear.
> 
> Also, the `used()` value is already captured in the event so we would have it duplicated.

> > For the liveness value to be useful it would have to be updated at each GC, and we need to investigate further to see how we can do that in a "cheap" way. I would prefer if G1 did just return used() in live() as a start and we can create a follow-up task to investigate how to best add a better estimate. Do you see any problem with this?
> 
> Actually yes - this event was meant to be a cheap way to see whether the _known_ live size is growing (has an upwards trend) which would using the `used()` value make more difficult and unreliable.
> 
> I would prefer keeping the implementation to return the lower bound of the live size as is the case for other GCs as well. May add explanatory comments to the code and the event definition to make this clear.
> 
> Also, the `used()` value is already captured in the event so we would have it duplicated.

Sure, but for the event to be useful we want the reported value to be as close to the reality as possible. I don't understand why you want the lower bound, can you explain why? I would go for the upper bound, which in that case would be `used()` at the end of the GC. I know `used()` is not perfect, but for G1 this is the best "cheap" value we have for liveness at the end of any GC. 

So as a middle road I would suggest to update `G1CollectedHeap::gc_epilogue(bool full)` to include:
set_live(used());

With this you don't need the changes for the `G1FullCollector`. The liveness calculated at Remark would be used until the next young collection and I think here is where some improvements could be made. During the mixed phase a better solution would make use of the liveness information we have for the old regions together with what is newly allocated, but this needs further investigation.

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

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