RFR (S): 8135200: Add White Box method that enumerates G1 old regions with less than specified liveness and collects statistics.

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Wed Sep 23 13:41:57 UTC 2015


Hi Thomas,

Thank you for reviewing this fix.

Please read comments inline.

Regards, Kirill

On 22.09.2015 16:17, Thomas Schatzl wrote:
> "Hi Kirill,
>
> On Wed, 2015-09-09 at 21:10 +0300, Kirill Zhaldybin wrote:
>> Hi!
>>
>> Could you please review a patch for JDK-8135200?
>>
>> I added White Box method that enumerates G1 old regions with less than
>> specified liveness and collects statistics.
>>
>> CR: https://bugs.openjdk.java.net/browse/JDK-8135200
>>
>> Webrev:
>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8135200/webrev.00/
>> hotspot:
>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8135200/hotspot/webrev.00/
>>
New Webrev:
http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8135200/webrev.01/
hotspot:
http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8135200/hotspot/webrev.01/
>
>    some comments about the change:
>
> - there is some wrong assumption in the code:
>
> 370         if (size == reg_size) { // non-full regions are not included
> to the mixed GC
>   371           totalMemToFree += size - prev_live;
>   372         }
>
> This is not true. G1 may try to evacuate non-full regions (regions
> recently allocated into), but it is just unlikely.
changed comments, since we don't need exact value, lowest estimation 
fits too.
>
> - lines 374-376 look like debug code. Are they required for some reason?
fixed, output deleted.
>
> As for the follow-up JDK-8129579 New tests for G1 Mixed GC are required"
> I am not sure if it is actually useful to try to test them. I mean,
> these are artifacts of the current implementation, and as long as mixed
> gc goals are met imo mixed gc may do anything to achieve its goal.
>
> Thanks,
>    Thomas
>
>




More information about the hotspot-gc-dev mailing list