RFR: 8134626 - Misc cleanups after generation array removal

[Jesper Wilhelmsson]

Hi Thomas,

Regarding younger_gen_as_roots I think the right thing to do here is to rename 
the parameter. I did this in the new version.

Also fixed all the comments you pointed out. Thanks!

http://cr.openjdk.java.net/~jwilhelm/8134626/webrev.01/

You probably prefer to look at
http://cr.openjdk.java.net/~jwilhelm/8134626/webrev.01/increment/

Thanks,
/Jesper


Den 3/9/15 kl. 14:24, skrev Thomas Schatzl:
> Hi Jesper,
>
> On Fri, 2015-08-28 at 16:37 +0200, Jesper Wilhelmsson wrote:
>> Hi,
>>
>> Please review this rather large cleanup. While working with the generation array
>> removal I did a bunch of unrelated cleanups in the code I passed by. I split
>> this out into a separate patch to keep the other changes clean.
>>
>> This change is split into three webrevs to make it easier to review. I intend to
>> push all three as one change.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8134626
>>
>>
>> Increment 1: Renaming
>> http://cr.openjdk.java.net/~jwilhelm/8134626/webrev.00/inc1-renaming/
>>
>> Changes the variable name gen to young_gen or old_gen depending on what the
>> variable will contain. The few cases where the name gen is untouched are places
>> where the variable actually can contain either generation.
>>
>
> looks good.
>
>>
>> Increment 2: Comments and indentation
>> http://cr.openjdk.java.net/~jwilhelm/8134626/webrev.00/inc2-comments_and_indentation/
>>
>> * Fixes up alignment in lots of places.
>> * Inserts and removes empty lines where needed.
>> * Cleans up comments where we previously talked about older and younger
>> generations. Fixes a few typos and clarifies some comments with respect to
>> young/old generations and collections.
>> * Added spaces around some operators.
>>
>
> concurrentMarkSweepGeneration.cpp, 2420, 3021, 5171, and 2492 (in the
> new code): the parameter is actually called younger_gen_as_roots, so the
> changes to young_gen_as_roots are wrong.
>
> concurrentMarkSweepGeneration.cpp, the comment in 2946 seems useless,
> because first it says "do X" and at the end "no, we actually don't do
> X".
>
> concurrentMarkSweepGeneration.inline.hpp 298: The sentence needs to be
> put into singular form, i.e. "If the young gen collection has been
> skipped, ..."
>
> parNewGeneration.cpp, 539: upper case "if" needed
>
> parNewGeneration.cpp, 983: "an" -> "and" (I think)
>
> heapRegionType.hpp, line 38: I think the brackets after "major type"
> should contain "young, old, humongous, archive" now
>
> defNewGeneration.inline.hpp, 60: the comment should be updated to
> reflect that we only have a single older generation now. I.e. "... p is
> also in the old generation, but..."
>
> Otherwise looks good. I do not need a re-review for this.
>
>>
>> Increment 3: Code changes
>> http://cr.openjdk.java.net/~jwilhelm/8134626/webrev.00/inc3-code_changes/
>>
>> * Merged strings that was split on several lines for no good reason.
>> * Added braces for if statements and for loops.
>> * Removed dead code.
>> * Moved variable initialization to initializer list in CollectedHeap constructor.
>> * Updated flag descriptions "youngest" -> "young".
>
> Looks good.
>
> Thomas
>
>