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

[Thomas Stüfe]

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?


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

I will prepare a change and also fix Solaris.

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