RFR (M): 8066780, 8066781, 8066782: Cleanup of duplicated code in TenuredGeneration and ConcurrentMarkSweepGeneration

[Bengt Rutisson]

Thanks Thomas and Kim for the reviews!

Thomas, I fixed all of your formatting comments before I pushed.

Bengt

On 2014-12-09 14:20, Thomas Schatzl wrote:
> Hi Bengt,
>
>    changes look good to me (sans Kim's suggestions) - following there are
> a few nitpicks about some minor code style issues that could be fixed.
> Feel free to ignore ones that cause too much effort.
>
> On Mon, 2014-12-08 at 16:20 +0100, Bengt Rutisson wrote:
>> On 2014-12-08 10:32, Bengt Rutisson wrote:
>>> Hi Kim,
>>>
>>> Thanks for looking at this!
>>>
>>> On 2014-12-05 20:07, Kim Barrett wrote:
>>>> On Dec 5, 2014, at 10:18 AM, Bengt Rutisson
>>>> <bengt.rutisson at oracle.com> wrote:
>>>>> I was looking at the duplicated code in TenuredGeneration and
>>>>> ConcurrentMarkSweepGeneration and wanted to clean it up. I decided
>>>>> to split the cleanup into three parts (thanks Kim for suggesting
>>>>> that in a pre-review). The parts are closely related so I think it
>>>>> is a good idea to review them together. Thus, I'm asking for reviews
>>>>> for all three changes in this same email.
>>>>>
>>>>> Here are the three changes:
>>>>>
>>>>> Split CardGeneration out to its own file
>>>>> https://bugs.openjdk.java.net/browse/JDK-8066780
>>>>> http://cr.openjdk.java.net/~brutisso/8066780/webrev.00/
>>>>>
>>>>> Just moving code from generations.cpp/hpp to cardGeneration.cpp/hpp.
>>>>>
> Not sure if you want to fix up some coding style issues too:
>
> - braces in cardGeneration.cpp:54,
> - capital initial letter for the comment of
> CardGeneration::_shrink_factor
>
>>>>> Minor cleanups to TenuredGeneration
>>>>> https://bugs.openjdk.java.net/browse/JDK-8066781
>>>>> http://cr.openjdk.java.net/~brutisso/8066781/webrev.00/
>>>>>
>>>>> Removing some dead code and making sure that the include guard in
>>>>> the inline file is correctly named. The _last_gc variable was
>>>>> exported to the SA agent but the SA agent never referenced it.
> - capital initial letter of comment for TenuredGeneration::_the_space
>
> Feel free to ignore.
>
>>>>> Move common code from CMSGeneration and TenuredGeneration to
>>>>> CardGeneration
>>>>> https://bugs.openjdk.java.net/browse/JDK-8066782
>>>>> http://cr.openjdk.java.net/~brutisso/8066782/webrev.00/
>>>>>
>>>>> This is the actual move of common code from TenuredGeneration and
>>>>> ConcurrentMarkSweepGeneration to CardGeneration. Both subclasses use
>>>>> a single Space instance and by allowing CardGeneration to find that
>>>>> space instance it was possible to move many of the methods that work
>>>>> on the Space instance up to the CardGeneration too.
> (I saw that comments in concurrentMarkSweepGeneration.hpp start with
> lower case letter. I think it's best to ignore this as these are a lot).
>
> - maybe the lines that only contain two forward slashes could be
> removed, in concurrentMarkSweepGeneration.hpp:175,177
>
> - Some copyright dates were not updated (I am not insisting on them, but
> apparently there has been put some effort in doing that, so it would be
> nice to do all of them), in particular in the .inline.hpp files.
>
> - maybe the empty lines in tenuredGeneration.cpp:260, 265, 285 could be
> removed.
>
> Thanks,
>    Thomas
>
>