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

[Coleen Phillimore]

For comments below, my answer is ok.   I like this change.  I might want 
to a constructor with a choice of where to allocate later, but this 
looks good.   It's a nice solution to your G1 malloc problems.

Coleen

On 4/5/2013 7:18 AM, Bengt Rutisson wrote:
>
> Hi Coleen,
>
> On 4/4/13 5:25 PM, Coleen Phillimore wrote:
>>
>> On 04/04/2013 07:32 AM, Bengt Rutisson wrote:
>>>
>>>
>>> Hi Colleen,
>>>
>>> Thank you so much for looking at this!
>>>
>>> On 4/1/13 11:04 PM, Coleen Phillimore wrote:
>>>>
>>>> Bengt,
>>>>
>>>> This is a nice way to solve this, although don't know why you don't 
>>>> just use mmap for all the task queues.   You don't allocate and 
>>>> deallocate them a lot, do you?
>>>
>>> Right, I was thinking about using mmap directly but when I looked at 
>>> the current usages of ::mmap I can only find that it is being used 
>>> in src/os files. So, it seems to me like this is not the API we are 
>>> supposed to use in the rest of the code. The next level of 
>>> abstraction is os::reserve_memory() and os::commit_memory(), so 
>>> that's why I picked them. It would have been nice if I could have 
>>> just used os::commit_memory(), but internally it uses MAP_FIXED 
>>> which means that I can't pass NULL as the addr parameter. So, I have 
>>> to use os::reserve_memory() first.
>>>
>>> Are you ok with these calls, or do you think I should change to use 
>>> mmap directly?
>>
>> Oh, no, I think you should use os::reserve_memory().   My question 
>> was why do you conditionally use reserve_memory vs. os::malloc() for 
>> these task queues.  Or are they always big enough that they'll end up 
>> using os::reserve_memory().
>
> Ah. Sorry, I misunderstood before.
>
> Good point. I think the task queues will always be large but I would 
> like the ArrayAllocator to be of more general use. We could use it for 
> structures that have a dynamic size and only need to be mmapped if 
> they are large enough.
>
>>>
>>>>
>>>> You can also use the trick in globals.hpp:
>>>>
>>>> *+   product(uintx, ArrayAllocatorMallocLimit, SOLARIS_ONLY(64K) NOT_SOLARIS(MAX_UINT),  \*
>>>> *+           "Allocation less than this value will be allocated "              \*
>>>> *+           "using malloc. Larger allocations will use mmap.")*
>>>>
>>>> And include both solaris sparc and x86 in the size calculation 
>>>> (since sparc malloc has the same problem as x86 just not as bad), 
>>>> and save editing all the globals_<os_cpu>.hpp files.
>>>
>>> Thanks! This is much better! It also means that I can use an 
>>> experimental flag, so I'll change to that.
>>
>> Yes, I guess it's better as an experimental flag.  It could be 
>> product also.  People might want their own sizes.
>
> I'd like to keep it experimental for a while to avoid that it is being 
> used too much.
>
> Thanks,
> Bengt
>
>>
>> Coleen
>>
>>>
>>>
>>> Thanks again for this feedback. It was exactly what I needed. I'll 
>>> send out an updated webrev and a "real" review request soon.
>>>
>>> Bengt
>>>
>>>>
>>>> Coleen
>>>>
>>>> On 03/28/2013 06:09 PM, 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:
>>>>>
>>>>> - I am adding a flag that has the same value on all platforms 
>>>>> except Solaris x86. There is the product_pd flag macro to support 
>>>>> this. But there is no experimental_pd marcro. I would have 
>>>>> preferred to make my new flag experimental. Should I add 
>>>>> experimental_pd or should I just use a product flag?
>>>>>
>>>>> - Even with product_pd I think I still have to go in to all the 
>>>>> different platform files and add the exact same code to give the 
>>>>> flag a default value on all platforms. Is there a way to have a 
>>>>> default value and only override it on Solaris x86?
>>>>>
>>>>> - 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?
>>>>>
>>>>> 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.
>>>>>
>>>>> When we run with ParallelOldGC we get three task queues per GC 
>>>>> thread. Each task queue takes mallocs 1MB. The failing machine in 
>>>>> the bug report has lots of CPUs and ends up with 83 GC threads. 
>>>>> This is 249 MB, which is more than we can get out of the 256 MB 
>>>>> limited C heap considering that there are other things that get 
>>>>> malloced too.
>>>>>
>>>>> So, the problems occur mostly on Solaris x86. My suggested fix 
>>>>> tries to address this by letting the task queues be mapped instead 
>>>>> of malloced on Solaris x86. Instead of inlining this logic in 
>>>>> taskqueue.cpp I added a more general class. The reason for this is 
>>>>> that I think we need to use the same logic in more places, 
>>>>> especially for G1, which is mallocing quite a lot.
>>>>>
>>>>> Since I think malloc on other platforms use mapped memory for 
>>>>> large malloc requests I think it is enough for this change to have 
>>>>> effect on Solaris. The other platforms probably have better 
>>>>> heuristics than I can come up with for which sizes should be 
>>>>> mapped. On Sparc we have the same limitation with malloc, but we 
>>>>> have more memory available for the C heap. This is why I have only 
>>>>> enabled this for Solaris x86.
>>>>>
>>>>> Also, I will be on vacation for a few days. Back in the office 
>>>>> Thrusday April 4. I'm happy for any feedback on this, but if I 
>>>>> don't respond until next week you know why :)
>>>>>
>>>>> Thanks,
>>>>> Bengt
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130405/d2aa379f/attachment.html