RFR 8067655: Clean up G1 remembered set oop iteration

Mikael Gerdin mikael.gerdin at oracle.com
Wed Dec 17 08:42:12 UTC 2014


Hi Kim,

On 2014-12-17 00:03, Kim Barrett wrote:
> On Dec 16, 2014, at 9:33 AM, Mikael Gerdin <mikael.gerdin at oracle.com> wrote:
>>
>> Hi all,
>>
>> while reading through the code which iterates over oops pointing into the collection set I noticed that the closure used to implement the iteration, G1ParPushHeapRSClosure, is hooked into the de-virtualization macros with oop_oop_iterate_nv but the static type is not preserved all the way down to the call site where oop_iterate is called, so the de-virtualization cannot work.
>>
>> I fixed the code to pass down the correct type and while doing that I had to clean up some dead code, such as the FilterKind parameter to the HeapRegionDCTOC which was only ever used with the IntoCSFilterKind. The filtering is not needed at all since G1ParPushHeapRSClosure already does a collection set check so I removed the filtering altogether.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8067655
>> Webrev: http://cr.openjdk.java.net/~mgerdin/8067655/webrev.0/
>>
>> Testing:
>> JPRT, jbb2005 on x86_64 and sparc.
>> The jbb runs showed no significant changes and the ScanRS time changes are inconclusive but I still think this is a nice cleanup change. At least this change makes it clear that it's actually only one closure type which is used by the HeapRegionDCTOC.
>>
>> /Mikael
>
> Ouch.  Nice find.  Looks good, other than the broken comment below.

Thanks for the review!

>
> ------------------------------------------------------------------------------
> src/share/vm/gc_implementation/g1/heapRegion.cpp
>    96     // Last object. Need to do pass a MemRegion here too.
>
> Broken comment.  Previous comment was
>   135     // Last object. Need to do dead-obj filtering here too.
>
> which I think was correct.

I guess both of the comments are correct but I'll revert to the previous 
comment.

(this seems like one of those "there was a bug here where we forgot to 
filter dead object, I'll add a comment")

/Mikael

>
> ------------------------------------------------------------------------------
>



More information about the hotspot-gc-dev mailing list