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

[Thomas Stüfe]

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
>>>
>>
>>
>
>