RFR: 8036025 - Sort the freelist in order to shrink the heap
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Tue Mar 18 15:07:47 UTC 2014
Hi Thomas,
Thanks for looking at this. New webrev here:
http://cr.openjdk.java.net/~jwilhelm/8036025/webrev.4/
Comments inline.
Thomas Schatzl skrev 18/3/14 13:31:
> 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().
Agreed and fixed.
> 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.
I wanted to call it just FreeRegionList::remove(bool) but for some reason the
windows build failed due to HeapRegionSetBase::remove(HeapRegion*), so I changed
the name for now.
> - g1CollectedHeap.cpp:600: the comment needs to be adapted. It talks
> about remove_head_or_null() still.
Fixed.
> - 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.
Is there really any guideline regarding coding style that is not generally
accepted in HotSpot? ;-)
Anyhow, I fixed it.
> - g1CollectedHeap.hpp:507: "humongous objet," -> "humongous object" :)
Didn't I fix this already?
Thanks,
/Jesper
>
> Thanks,
> Thomas
>
>
More information about the hotspot-gc-dev
mailing list