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

[Coleen Phillimore]

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 <mailto: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/
>         <http://cr.openjdk.java.net/%7Estuefe/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
>
>
>