RFR 8067655: Clean up G1 remembered set oop iteration

[Jon Masamitsu]

http://cr.openjdk.java.net/~mgerdin/8067655/webrev.0/src/share/vm/gc_implementation/g1/heapRegion.hpp.frames.html

Are we trying to move toward all parameters on 1 line or
1 line per parameter?

   84   HeapRegionDCTOC(G1CollectedHeap* g1,
   85                   HeapRegion* hr, G1ParPushHeapRSClosure* cl,
   86                   CardTableModRefBS::PrecisionStyle precision);


Same here.

http://cr.openjdk.java.net/~mgerdin/8067655/webrev.0/src/share/vm/gc_implementation/g1/heapRegion.cpp.frames.html

   50 HeapRegionDCTOC::HeapRegionDCTOC(G1CollectedHeap* g1,
   51                                  HeapRegion* hr, G1ParPushHeapRSClosure* cl,
   52                                  CardTableModRefBS::PrecisionStyle precision) :


Otherwise, looks good.

Jon

On 12/16/2014 06:33 AM, Mikael Gerdin 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