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

[Erik Helin]

On 2016-07-22, Mikael Gerdin wrote:
> Hi,
> 
> On 2016-07-21 17:11, Thomas Schatzl wrote:
> >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.
> 
> +1

Thanks for your reviews Mikael and Thomas!

Erik

> /Mikael
> 
> >
> >Thomas
> >