RFE (m) (Prelminary): JDK-7197666: java -d64 -version core dumps in a box with lots of memory

[Coleen Phillimore]

Bengt,
One comment below about the error message.

On 04/04/2013 08:11 AM, Bengt Rutisson wrote:
>
> Hi Thomas,
>
> Thanks for looking at this!
>
> On 3/29/13 10:33 AM, Thomas Schatzl wrote:
>> Hi,
>>
>> On Thu, 2013-03-28 at 23:09 +0100, Bengt Rutisson wrote:
>>> Hi all,
>>>
>>> Sending this to both runtime and GC since I think it concerns both
>>> areas.
>>>
>>> I'd like some feedback on this preliminary change. I still want to do
>>> some more testing and evaluation before I ask for final reviews:
>>>
>>> http://cr.openjdk.java.net/~brutisso/7197666/webrev.00/
>>>
>>> In particular I would like some feedback on these questions:
>>>
>>> - The class I am adding, ArrayAllocator, wants to choose between doing
>>> malloc and mmap. Normally we use ReservedSpace and VirtualSpace to get
>>> mapped memory. However, those classes are kind of clumsy when I just
>>> want to allocate one chunk of memory. It is much simpler to use the
>>> os::reserve_memory() and os::commit_memory() methods directly. I think
>>> my use case here motivate using these methods directly, but is there
>>> some reason not to do that?
>> The *Space classes are heavily geared to be used as virtual memory
>> management helpers for a two-generation heap, so I'm not sure if they
>> are appropriate here.
>>
>> Not sure why you chose to use AllocateHeap(), I always thought that the
>> use of the corresponding macros (NEW_C_HEAP_ARRAY etc.) is recommended.
>
> The reason I use AllocateHeap() directly is that I already have the 
> size calculated and if I use NEW_C_HEAP_ARRAY it would have to do that 
> calculation again.
>
>> The error messages of ArrayAllocator give very little information, maybe
>> add the requested block size that lead to the failure in the message.
>
> The requested size is part of the error message. Is there more you 
> would like to have in the message? Currently it looks like this:
>
> # There is insufficient memory for the Java Runtime Environment to 
> continue.
> # Native memory allocation (malloc) failed to allocate 1048576 bytes 
> for Allocator failed to commit
>
> I admit that this message is not perfect. First it says "(malloc)" 
> even though it was actually mmap that failed. But this is I think more 
> of a bug in vm_exit_out_of_memory() than in my use of it. There are 
> several places that use VirtualSpace and when it fails call 
> vm_exit_out_of_memory(). This has the same issue as my code. For 
> example when we try to expand the heap in G1 with 
> G1CollectedHeap::expand()
>
> I also think the last part of the message looks a bit odd now that I 
> see the whole message. But I would like to know if it was reserve or 
> commit that failed. Do you think I should change to "Allocator 
> (reserve)" and "Allocator (commit)" instead?
>

I added (malloc) to the error message a while ago because I didn't 
realize that mmap goes through vm_exit_out_of_memory() or at least I 
hadn't seen it from that path at the time. Passing a flag to 
vm_exit_out_of_memory() that it was an mmap that failed rather than 
malloc would be better.   This could be done as a separate bug fix.   
What do you think?

Coleen

>> There is a typo in the comment "uses malpped memory" -> "uses mapped
>> memory".
>
> Thanks! Fixed.
>
>> While it is not strictly necessary, you could initialize all member
>> variables in the constructor's initialization list.
>
> OK. Done.
>
>>
>> Maybe there is a common name for such type of objects, i.e. cleanup
>> helpers that are "stack allocated" only to be automatically called
>> during destructor of the enclosing class in c++ jargon.
>> It seems to be a common pattern.
>>
>> If that is not the case, maybe rename it to LargeArrayAllocator to
>> better indicate the purpose in addition to the comment.
>
> I don't really like LargeArrayAllocator since this one can be used for 
> arrays that can be both small and large. The whole idea is that when 
> we have dynamic sizes we need a simple way to allocate both small and 
> large arrays but get them allocated in the right way. That's why I 
> picked just ArrayAllocator.
>
>>> Some background on the change:
>>>
>>> The default implementation of malloc on Solaris has several limitation
>>> compared to malloc on other platforms. One limitation is that it can
>>> only use one consecutive chunk of memory. Another limitation is that
>>> it always allocates in this single chunk of memory no matter how large
>>> the requested amount of memory is. Other malloc implementations
>>> normally use mapped memory for large allocations.
>>>
>>> The Java heap is mapped in memory and we try to pick a good address
>>> for it. The lowest allowed address is controlled by
>>> HeapBaseMinAddress. This is only 256 MB on Solaris x86 (other
>>> platforms have at least 2 GB). Since the C heap ends up below the Java
>>> heap it means that in some cases it is limited to 256 MB.
>> 256 MB seems to be very low in any case. Maybe in addition to that, make
>> HeapBaseMinAddress dependent on the OS?
>
> HeapBaseMinAddress is already dependent on the OS. It is explicitly 
> set to 256MB on Solaris x86 and at least 2GB on all other platforms. 
> We want to keep it to 256MB on Solaris x86 to be able to use as large 
> heaps as possible for compressed oops.
>
>>
>> Is there some documentation/FAQ that tells solaris/x86 users to consider
>> increasing HeapBaseMinAddress if they run out of C heap?
>
> That's a good point. I can talk to SE about how they would like to 
> communicate this to users.
>
> Thanks,
> Bengt
>
>> Thomas
>>
>