RFR(xs): 8079449: Improve os::attempt_reserve_memory_at() fallback coding on Linux, BSD, Solaris

[Coleen Phillimore]

Hi Thomas.
Ok, I'm fine with the original change.  We need someone else to review it also.
Thanks
Coleen

Sent from my iPhone

> On May 20, 2015, at 10:37 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> 
> Hi Coleen,
> 
> I took a close look at Solaris and would prefer for now to keep Solaris out of this patch. It is too different from the linux way of allocation (e.g., default addresses for subsequently placed memory mappings go downward, not upward like on linux and bsd) to be an easy fix.
> 
> I did test Solaris and it seems that by default, mmap returns the requested address, so I think this fallback coding gets seldom exercised (tested on Solaris 5.11 on x64). 
> 
> I think it makes sense to put os::attempt_reserve_memory_at into os_posix.cpp, but would prefer this to be a separate RFE. I would like to understand the history of this code a bit more, especially on Solaris, before attempting this fix.
> 
> Kind Regards, Thomas
> 
> 
> 
>> On Tue, May 12, 2015 at 7:19 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>> 
>> See below.
>> 
>>> On 5/12/15, 6:27 AM, Thomas Stüfe wrote:
>>> Hi Coleen,
>>> 
>>> thank you for looking at this change!
>>> 
>>> Comments inline.
>>> 
>>>> On Mon, May 11, 2015 at 11:31 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> I think this fallback code may be used for CDS (class data sharing) to get the archive address.  I also think Solaris is more likely to not give a requested address (solaris 10 doesn't at all).
>>> 
>>> I also think BSD (macos at least) completely ignores the req_addr parameter. Adresses MacOS returns feel totally random, might be a security feature.
>>>  
>>>> Since we haven't observed a performance problem with this code on solaris, I don't think it should be fixed, unless you have such a test case.
>>> 
>>> We saw unreproducable, rare crashes on Linux Itanium -  weird sudden deaths without cores/hs-err files - during tests which really stress os::attempt_reserve_memory_at(). First I suspected some hidden error with the many unmap() calls in the fallback code, but code review did not show errors. However, when we disabled the fallback coding, the errors went away. There are some theories to that, one is that the fact that we reserve ten times the original size in the fallback coding somehow triggers an OOM killer. But we were never able to prove anything.
>>> 
>>> However, even without understanding that bug, I think my patch is an easy improvement.
>>>  
>>>> Did you test this on 32 bit linux with CDS?  (java -Xshare:dump, java -Xshare:on -version) I think you might see this code in action.
>>> 
>>> I can see this fallback coding making much more sense on 32bit. Why not restrict it to 32bit?
>> 
>> I don't know...
>>>  
>>>> The patch looks good though, and I believe it fixes an NMT double tracking bug (which we also haven't seen) with               os::pd_attempt_reserve_memory_at calling os::reserve_memory.   It's worth fixing with your patch for that reason.   Can you also fix this call for solaris?
>>> 
>>> Would be nice to take credit for this, but there was no NMT double tracking :-) - os::reserve_memory() was coupled with os::unmap_memory(), both do NMT tracking. But the tracking was unnecessary, therefore I changed the calls to anon_mmap/anon_munmap.
>> 
>> Oh, yes, you're right, there's double tracking then double untracking.
>>> 
>>> I will prepare a change and also fix Solaris.
>> 
>> I wonder if the Solaris and linux code can be merged then into os_posix.cpp.   I don't know why they are different except they were probably written at different times to do the same thing.  I could do some archeology on this.
>> 
>> Thanks for taking this on.
>> Coleen
>>> 
>>> Thanks, Thomas
>>> 
>>>  
>>>> 
>>>> Thanks,
>>>> Coleen
>>>> 
>>>> 
>>>>> On 5/6/15, 10:07 AM, Thomas Stüfe wrote:
>>>>> Hi all,
>>>>> 
>>>>> please take a look at the following patch:
>>>>> 
>>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8079449/webrev.00/webrev/
>>>>> https://bugs.openjdk.java.net/browse/JDK-8079449
>>>>> 
>>>>> More details are in my original mail:
>>>>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2015-April/014600.html
>>>>> 
>>>>> 
>>>>> This patch adds a fix to fallback coding used in
>>>>> os::attempt_reserve_memory_at() on Linux and BSD.
>>>>> 
>>>>> If this patch meets with approval, I will port the fix to Solaris, but
>>>>> Solaris implementation was just different enough for this to be not
>>>>> straightforward. Therefore I'd like to get some feedback for this issue
>>>>> first.
>>>>> 
>>>>> Note - as I wrote in my original mail - that I have my doubts that this
>>>>> fallback coding is even needed at all - in tests I never saw it doing
>>>>> anything useful. I'd actually rather just remove it than fix it. What do
>>>>> you think?`
>>>>> 
>>>>> Kind Regards, Thomas
>