CRR: 7045662: G1: OopsInHeapRegionClosure::set_region() should not be virtual (XS)

[Tony Printezis]

Stefan and John,

First, thanks for taking a look at this so promptly!

Yes, good catch. I had missed IntoCSOopClosure. It's gone too! Along 
with VerifyRSCleanCardOopClosure which is not used either (this is yet 
another problem with closures: you remove the code but it's very easy to 
leave the closures behind....).

Anyway, here's the updated webrev:

http://cr.openjdk.java.net/~tonyp/7045662/webrev.1/

And I'm all set with this one!

Tony

On 6/1/2011 8:01 AM, Stefan Karlsson wrote:
> What about IntoCSOopClosure? It also declares a set_region method. But 
> since it's not used anywhere, we should probably just remove it.
>
> Otherwise, this looks good.
>
> StefanK
>
> On 06/01/2011 01:40 PM, Tony Printezis wrote:
>> Hi,
>>
>> (first of a series of code review requests for changes I've been 
>> recently working on)
>>
>> Could I please have a couple of code reviews for this simple change?
>>
>> http://cr.openjdk.java.net/~tonyp/7045662/webrev.0/
>>
>> The main change is to make the set_region() method non-virtual and 
>> remove two closures that were the reason for that method being 
>> virtual but that are not used any more. This change shows a very 
>> small (~1%) but measurable improvement in GC times on some platforms.
>>
>> I also piggy-backed a couple of extra minor fixes on this change: 
>> removal of a third unused closure and fixing two small typos in the 
>> copyright text on two files (I have no idea how that C become 
>> lowercase!).
>>
>> Tony
>>
>