RFR: 8134626 - Misc cleanups after generation array removal
Thomas Schatzl
thomas.schatzl at oracle.com
Thu Sep 3 12:24:17 UTC 2015
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
More information about the hotspot-gc-dev
mailing list