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

[Poonam Bajaj]

Looks good to me!

Thanks,
Poonam


On 6/2/2011 3:31 AM, Tony Printezis wrote:
> 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
>>>
>>