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

[Thomas Schatzl]

Hi Erik,

On Thu, 2016-07-21 at 16:26 +0200, Erik Helin wrote:
> On 2016-07-19, Thomas Schatzl wrote:
> > 
> > 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. 
[...]
> > > 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.
> Thanks, fixed, below is the incremental patch:
> 
> diff -r e19783ebbecf src/share/vm/gc/g1/g1DefaultPolicy.cpp
> --- a/src/share/vm/gc/g1/g1DefaultPolicy.cpp    Mon Jul 18 14:05:41
> 2016 +0200
> +++ b/src/share/vm/gc/g1/g1DefaultPolicy.cpp    Thu Jul 21 16:16:47
> 2016 +0200
> @@ -99,3 +99,3 @@
>  
> -class G1YoungLengthPredictor {
> +class G1YoungLengthPredictor VALUE_OBJ_CLASS_SPEC {
>    const bool _during_cm;

Okay.

[...]
> On 2016-07-19, Thomas Schatzl wrote:
> > 
> >   - 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?
> I don't know why the policy thinks object copying is more expensive
> during concurrent mark, most likely this was the case waaaay back but
> is no longer true. I filed JDK-8162109 [0].
> 
> As to the effect of this patch, we shouldn't notice any difference.
> This is most likely the first time we have observed this behavior
> (otherwise the assert should have triggered), so we have probably
> never run into this situation before :) Remember that this bug only
> happens when a concurrent mark cycle finishes during the calculation
> of the eden length (not common).

  thanks. Looks good.

Thomas