Request for review (s): 6814390 G1: remove the concept of non-generational G1

[Bengt Rutisson]

Hi Ramki,

On 2011-08-18 10:10, Y. Srinivas Ramakrishna wrote:
> Hi Bengt -- that should also make the method checkpointRootsInitial() 
> dead, and
> thus also record_concurrent_mark_init_start() [The clue is the guarantee
> you deleted in the latter method at line 940 which would be violated 
> in the case of G1Gen
> because it would be tautologically false now.] I think the dead code 
> contagion
> does stop there, as far as i could tell from a browse of the code. 
> Perhaps a good
> IDE may find you a few more dead methods, who knows...

Wow! This change really propagates. Thanks for catching this. I found a 
couple of more methods to delete and one class. Now I hope I got all of 
the dead code that this change generated.

This is what I deleted this time around:

G1CollectorPolicy::record_concurrent_mark_init_start()
G1CollectorPolicy::record_concurrent_mark_init_end()
ConcurrentMark::checkpointRootsInitial()
G1CollectedHeap::do_sync_mark()
class CMMarkRootsClosure

Here is an updated webrev:
http://cr.openjdk.java.net/~brutisso/6814390c/webrev.03/

Thanks,
Bengt

>
> Rest looks good to me.
> -- ramki
>
> On 8/18/2011 12:28 AM, Bengt Rutisson wrote:
>>
>> Hi John,
>>
>> Thanks for the review!
>>
>>> Looks good to me. You may want to consider removing the 
>>> CMCheckpointRootsInitialClosure - it's only instantiated in the code 
>>> you removed from concurrentMarkThread.cpp.
>>
>> Good point. I removed the CMCheckpointRootsInitialClosure class. Here 
>> is an updated webrev:
>>
>> http://cr.openjdk.java.net/~brutisso/6814390c/webrev.02/
>>
>> Bengt
>>
>>>
>>> JohnC
>>>
>>> On 08/16/11 11:03, Bengt Rutisson wrote:
>>>>
>>>> Hi all,
>>>>
>>>> Could I have a couple of reviews for this change?
>>>>
>>>> Background:
>>>> G1 was originally designed to be able to run in a non-generational 
>>>> mode. This has not been used for a long time and no testing has 
>>>> been done. Thus, the code has bit rotted. We don't see the need for 
>>>> this feature anymore, so rather than fixing it we should remove it.
>>>>
>>>> This is actually a low priority item, but since it will make my 
>>>> next step (supporting young space sizing better) simpler I would 
>>>> like to get this CR out of the way.
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~brutisso/6814390c/webrev.01/
>>>>
>>>> CR:
>>>> 6814390 G1: remove the concept of non-generational G1
>>>> http://monaco.us.oracle.com/detail.jsf?cr=6814390
>>>>
>>>> Thanks,
>>>> Bengt
>>>>
>>>
>>
>