RFR: 8033923: Use BufferingOopClosure for G1 code root scanning

[Stefan Karlsson]

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
>