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

[Mikael Gerdin]

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

/Mikael

>
> Thomas
>