RFR: 8159464: DumpHeap.java hits assert in G1 code

[Thomas Schatzl]

Hi,

On Mon, 2016-07-18 at 15:59 +0200, Erik Helin wrote:
> Hi all,
> 
> this patch fixes an issue with the young gen sizing code in G1. The
> problem is that the G1 policy uses a binary search to come up with
> the
> young gen size, and the context for this binary search might change
> while it is running. For this particular bug, a concurrent mark cycle
> ended while the binary search was running. To fix this, we must save
> a
> copy of all the state needed for the search.
> 
> I've looked through the code and as far as I can see the only data
> that
> might change is the collector state (and the during_cm() state in
> particular). The fix is to save the value of
> collector_state()->during_cm() before the binary search. The data in
> G1Analytics is not updated while the young gen sizing code is
> running.
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8159464
> 
> Patches:
> I split the patch into two steps to make it a bit easier to review:
> 1. Add const to a lot of variables to ensure that they don't change:
>    http://cr.openjdk.java.net/~ehelin/8159464/00/add-const/
> 2. Move G1DefaultPolicy::predic_will_fit into its own class,
>    G1YoungLenghtPredictor. The G1YoungLengthPredictor class takes the
>    needed parameters in its constructor:
>    http://cr.openjdk.java.net/~ehelin/8159464/00/fix-bug/

  - G1YoungLengthPredictor needs a VALUE_OBJ_CLASS_SPEC or something
similar.

  - looking at the code, I think using an extra class to hold the
repeatedly passed parameters is quite nice.

  - this is just some question about the impact of the change of
during_concurrent_mark while we are calculating a new target length. I
would assume that the resulting young length during mark would at most
be smaller than the one determined later.

I.e. at most we would do a young gc a little bit too early, assuming
that for some reason object copy is a bit more expensive during that
time? Not sure about the logic for having different costs during and
ouside marking right now, do you remember?

Any concerns from you about this?

Thanks,
  Thomas