RFR: 8033923: Use BufferingOopClosure for G1 code root scanning

[Bengt Rutisson]

Hi Stefan,

The latest webrev looks good.

One minor nit:

We can remove this check since scan_rs always is non-NULL:

5166   if (scan_rs != NULL) {

Bengt


On 2014-02-10 10:49, Stefan Karlsson wrote:
>
> On 2014-02-07 16:38, Stefan Karlsson wrote:
>> On 2014-02-07 16:04, Mikael Gerdin wrote:
>>> Stefan,
>>>
>>> On Friday 07 February 2014 14.23.16 Stefan Karlsson wrote:
>>>> Hi all,
>>>>
>>>> Please review this patch to use BufferingOopClosure for the code root
>>>> scanning in G1.
>>>>
>>>> Webrev:
>>>>    http://cr.openjdk.java.net/~stefank/8033923/webrev.00
>>> There is some time accounting in G1RemSet::scanRS
>>>
>>> When you call buf_scan_non_heap_roots.done() in 
>>> g1_process_strong_roots we may
>>> miss accounting some of the time spent handling code roots from 
>>> regions.
>>>
>>> I think you need to pass down the buf_scan_non_heap_roots closure 
>>> all the way
>>> down to ScanRSClousre::scan_strong_code_roots and construct the
>>> CodeBlobToOopClosure there to be able to call .done() before the 
>>> update of
>>> _strong_code_root_scan_time_sec.
>>>
>>> Alternatively you may be able to add the closure application time 
>>> from after
>>> the extraction of the "object copying" data to the scan_rs 
>>> phase_time after
>>> the done()-call.
>>
>> OK. I see what you mean.
>>
>> I should probably only use the BufferingOopClosure for the 
>> process_strong_roots call, and use a non-buffering 
>> OopClosure/CodeBlobToOopClosure pair for the 
>> oops_into_collection_set_do call.
>
> New webrev:
> http://cr.openjdk.java.net/~stefank/8033923/webrev.01
>
> Changes from webrev.00:
> - Changed so that we don't use the buffering closures for 
> oops_into_collection_set_do.
> - Removed obsolete comment.
> - Removed unnecessary assert.
>
> thanks,
> StefanK
>
>>
>>
>> thanks,
>> StefanK
>>
>>>
>>> /Mikael
>>>
>>>> RFE:
>>>>    https://bugs.openjdk.java.net/browse/JDK-8033923
>>>>
>>>> The patch builds upon the changes in:
>>>>    RFR: 8033764: Remove the usage of StarTask from BufferingOopClosure
>>>> mail.openjdk.java.net/pipermail/hotspot-gc-dev/2014-February/009364.html 
>>>>
>>>>
>>>> thanks,
>>>> StefanK
>>
>