RFR: 8036025 - Sort the freelist in order to shrink the heap

Thomas Schatzl thomas.schatzl at oracle.com
Tue Mar 18 12:31:09 UTC 2014


Hi Jesper,

  thanks for updating the change with the latest state of the repo.

On Tue, 2014-03-18 at 12:51 +0100, Jesper Wilhelmsson wrote:
> Hi,
> 
> Recent changes in the GC code lead to non-trivial merge conflicts. I have 
> updated the change and here is a new webrev:
> 
> http://cr.openjdk.java.net/~jwilhelm/8036025/webrev.3/
> 
> The change itself should be the same but several name changes and cleanups in 
> related code was made that caused the conflicts.

  looks good. I agree with your suggestion in the other mail to postpone
better encapsulation for later.

Some minor nits:

- g1CollectedHeap.cpp:575: the comment is not really required here. I do
not see a reason to document that remove_region() call particularly.
Also this is imo already documented by the
FreeRegionList::remove_region().

Not sure if we need the "_region" suffix in that method's name as others
do not have it either. Maybe "remove_head_or_tail()" or just "remove",
or just keep it, as you wish.

- g1CollectedHeap.cpp:600: the comment needs to be adapted. It talks
about remove_head_or_null() still.

- g1CollectedHeap.cpp:6419, 6479: the generally accepted guideline for
parameter passing when it does not fit into a line is to use an extra
line for each of the parameters.

- g1CollectedHeap.hpp:507: "humongous objet," -> "humongous object" :)

Thanks,
  Thomas





More information about the hotspot-gc-dev mailing list