RFR (S): 8144714 Add extension point to G1 evacuation closures

[Mikael Gerdin]

On 2015-12-09 15:27, Jon Masamitsu wrote:
>
>
> On 12/08/2015 04:59 AM, Mikael Gerdin wrote:
>> Hi Jon,
>>
>> On 2015-12-04 18:42, Jon Masamitsu wrote:
>>> Mikael,
>>>
>>> Looks good.   Couple of minor points.
>>>
>>> http://cr.openjdk.java.net/~mgerdin/8144714/webrev.0/src/share/vm/gc/g1/g1InCSetState.hpp.frames.html
>>>
>>>
>>>
>>> Is the Ext value explained by the comment "The other values ..."? If it
>>> does apply
>>> to the Ext value, I don't understand.  "getting to the next generation"
>>> doesn't seem
>>> to apply.  Actually, I don't understand that comment with respect to
>>> Humongous either.
>>>
>>>    56     // The other values are simply encoded in increasing
>>> generation order, which
>>>    57     // makes getting the next generation fast by a simple
>>> increment.
>>> 58 Ext = -2, // Extension point
>>
>> The comment appears to be a bit outdated, I suggest changing it to:
>> // Selection of the values were driven to micro-optimize the encoding and
>> // frequency of the checks.
>> // The most common check is whether the region is in the collection
>> set or not,
>> // this encoding allows us to use an > 0 check.
>> // The positive values are encoded in increasing generation order, which
>> // makes getting the next generation fast by a simple increment. They
>> are also
>> // used to index into arrays.
>> // The negative values are used for objects requiring various special
>> cases,
>> // for example eager reclamation of humongous objects.
>
> Thanks for the update of the comment.  Looks good.

Thanks Jon!

/Mikael

>
> Jon
>
>>
>>>
>>>
>>> http://cr.openjdk.java.net/~mgerdin/8144714/webrev.0/src/share/vm/gc/g1/g1ParScanThreadState_ext.cpp.frames.html
>>>
>>>
>>>
>>> Lines 39 and 40 not particularly useful?
>>
>> FWIW, I understood that you meant the empty lines, I'll remove them. :)
>>
>> /Mikael
>>
>>>
>>> 37 template void G1ParScanThreadState::do_oop_ext<oop>(oop* ref);
>>> 38 template void G1ParScanThreadState::do_oop_ext<narrowOop>(narrowOop*
>>> ref);
>>> 39
>>> 40
>>>
>>> Jon
>>>
>>>
>>> On 12/4/2015 7:18 AM, Mikael Gerdin wrote:
>>>> Hi all,
>>>>
>>>> Please review this small change to add a hook in the G1 evacuation
>>>> path.
>>>> This allows extension code to register regions which are otherwise
>>>> interesting to the GC and get notifications when pointers into such
>>>> regions are encountered.
>>>>
>>>> Testing: JPRT, gc test suite
>>>> Performance:
>>>> This additional branch in G1ParScanClosure introduces a slight
>>>> regression of about 1-2% in GC pause times. I have some performance
>>>> fixes to make up for that regression in the pipe.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~mgerdin/8144714/webrev.0/
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8144714
>>>>
>>>> Thanks
>>>> /Mikael
>>>
>>
>