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

[Jon Masamitsu]

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.

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
>>
>