CRR (M): 7127697: G1: remove dead code after recent concurrent mark changes

[Tony Printezis]

Bengt,

Thanks for looking at it. Inline.

On 03/27/2012 08:19 AM, Bengt Rutisson wrote:
>
> Hi Tony,
>
> It is always difficult to review removed code, but this looks good to me.

Thanks.

> I would like to include your second change as well. It would be good 
> to remove BitMap::mostly_disjoint_range_union(). I realize it is a 
> difficult method to re-implement, but leaving it in without any usage 
> will make it break sooner or later anyway. So, better to re-implement 
> it if we ever need it again.

Well, re-implementing it is a bit of a waste of time. Maybe, it could be 
revived from the hg history...

> A couple of minor comments:
>
> concurrentMark.cpp
>
> Comment starting on line 2832 needs to be updated now that 
> drainAllSATBBuffers is gone. Maybe the whole comment is irrelevant?
>
> // This note is for drainAllSATBBuffers and the code in between.
> // In the future we could reuse a task to do this work during an
> // evacuation pause (since now tasks are not active and can be claimed
> // during an evacuation pause). This was a late change to the code and
> // is currently not being taken advantage of.

Thanks for pointing this out. It turns out that the method is actually 
unused now, so it's also gone along with the comment. ;-)

> heapRegion.hpp
>
> You removed CompleteMarkCSetClaimValue = 6 from enum ClaimValues. Any 
> reason why you did not renumber the entries following it?

Laziness? :-)

> In fact, why do the entries have numbers at all since they are 
> starting on 0 and just increasing by 1. That is the default for an 
> enum, right?

Not sure why it was done this way. It's nice to have the numbers, say, 
during debugging (so you don't have to count to find out what a value of 
'3' means). I left them as is and I renumbered the last three.

Thanks again for the code review! The new webrevs are here:

http://cr.openjdk.java.net/~tonyp/7127697/webrev.1/webrev.0.G1CMCleanup/

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

Tony

> Bengt
>
> On 2012-03-27 11:29, Tony Printezis wrote:
>> Hi all,
>>
>> I'd like a couple of code reviews for this set of changes:
>>
>> http://cr.openjdk.java.net/~tonyp/7127697/webrev.0/webrev.0.G1CMCleanup/
>>
>> which removes lots code from conc mark that was made unreachable from 
>> a recent set of changes. The changes are mostly code removal with 
>> only a very small number of code actually modified.
>>
>> This is an additional proposed change:
>>
>> http://cr.openjdk.java.net/~tonyp/7127697/webrev.0/webrev.1.G1CMCleanupExtra/ 
>>
>>
>> to remove a method from the bitmap class which is not used any more 
>> (it was only used by the code in G1 conc mark that I'm removing). I'd 
>> like some feedback on whether I should go ahead and remove it or 
>> leave it in in case it's used in the future.
>>
>> Tony
>>
>