RFR(s): 8077417: Cleanup of Universe::initialize_heap()

[Kim Barrett]

On Apr 15, 2015, at 12:58 PM, Per Liden <per.liden at oracle.com> wrote:
> 
>> On 13 Apr 2015, at 07:42, Kim Barrett <kim.barrett at oracle.com> wrote:
>> ------------------------------------------------------------------------------ 
>> src/share/vm/memory/universe.cpp 
>> 727   else { // UseSerialGC
>> 
>> Add a guarantee that UseSerialGC is true here?
> 
> It turns out that we have cases were no gc it selected, and then we fall back to using serial. There's a bug to fix that:
> 
> JDK-8068582: UseSerialGC not always set up properly
> 
> I'll add a comment in the code to say that we can't assert there. Will also add a comment to that bug to add an assert/guarantee when that bug is fixed.

Oh, right.  I’d forgotten about that bug.  Yes to your action plan.

>> src/share/vm/memory/universe.cpp 
>> 735   ThreadLocalAllocBuffer::set_max_size(Universe::heap()->max_tlab_size());
>> 
>> This used to be done before Universe::heap()->initialize().  Now it is
>> being called after the heap initialization.  Is that change of order
>> ok?
>> 
>> I looked at it and it seems ok, in which case I think the change of
>> order is actually an improvement.
> 
> I looked at when I did the patch and concluded that it was ok. In fact, it seemed a bit strange to set that up before calling initialize().

That’s what I thought too.

>> While I was looking around I noticed what might be a separate problem.
>> It seems possible with a sufficiently large heap that G1's
>> max_tlab_size computation might exceed the int[Integer.MAX_VALUE] size
>> limit discussed in CollectedHeap::max_tlab_size. I'm not sure whether
>> that limit is actually relevant to G1 though.  If it is relevant then
>> a new CR should be filed.
> 
> I don't think there's a problem there, G1's max_tlab_size will return (region_size_in_words / 2) - 1 words, and G1's region size at most 32M at the moment. Should be way below JINT_MAX.

After looking further, I agree there’s not a problem there.