RFR(S): 8165315: [ppc] Port "8133749: NMT detail stack trace cleanup"

[Lindenmaier, Goetz]

Thanks, David!
Best regards,
  Goetz

> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Montag, 12. September 2016 03:08
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Chris Plummer
> <chris.plummer at oracle.com>; hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR(S): 8165315: [ppc] Port "8133749: NMT detail stack trace
> cleanup"
> 
> Thanks Goetz! I like this version much better! :)
> 
> Cheers,
> David
> 
> On 9/09/2016 7:21 PM, Lindenmaier, Goetz wrote:
> > Hi Chris,
> >
> > I changed current_frame() and it's finally through our tests.  (We had an
> > issue with our build machines the day before ...)
> >
> > Here the new webrev:
> > http://cr.openjdk.java.net/~goetz/wr16/8165315-
> fixStackTrace/02/webrev.bs/
> > http://cr.openjdk.java.net/~goetz/wr16/8165315-
> fixStackTrace/02/webrev.hs/
> >
> > The webrev of the base repo is unchanged except for the reviewer
> attribution.
> >
> > I would appreciate a lot if you could sponsor this change.
> >
> > Best regards,
> >   Goetz.
> >
> >> -----Original Message-----
> >> From: Chris Plummer [mailto:chris.plummer at oracle.com]
> >> Sent: Dienstag, 6. September 2016 18:51
> >> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
> >> dev at openjdk.java.net
> >> Subject: Re: RFR(S): 8165315: [ppc] Port "8133749: NMT detail stack trace
> >> cleanup"
> >>
> >> Hi Goetz,
> >>
> >> Comments inline below:
> >>
> >> On 9/5/16 12:28 AM, Lindenmaier, Goetz wrote:
> >>> Hi,
> >>>
> >>> The test coming with 8133749 showed a row of problems on the ppc
> >> platforms.
> >>> This change fixed these.
> >>>
> >>>
> >>> Also, I moved "isSlowDebugBuild()" to Platform.java as there also is
> >>>
> >>> isDebugBuild(), and we have other places where we use this.
> >> Ok.
> >>>
> >>> Please review this change. I also please need a sponsor.
> >>> http://cr.openjdk.java.net/~goetz/wr16/8165315-
> >> fixStackTrace/01/webrev.bs/
> >>> http://cr.openjdk.java.net/~goetz/wr16/8165315-
> >> fixStackTrace/01/webrev.hs/
> >>>
> >>> More details:
> >>>
> >>> On power, current_frame() returns the frame of the method that
> >>> called current_frame(). This is as documented in os.hpp.
> >>>
> >>> Get_native_stack() in os_posix.cpp expects current_frame() to go up
> >>> one more frame. To adapt to this expectation, we increment toSkip by
> >>> one on ppc, which has the same effect. (If we change current_frame(),
> >>> one less frame will be printed to hs_err files etc.)
> >> The fact that os::current_frame() actually returns the caller's frame
> >> (two frames up from os:current_frame instead of one frame up) is
> covered
> >> by the following CR:
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8163900
> >> JDK-8163900 os::current_frame has a misleading name
> >>
> >> I would prefer that you changed PPC/AIX to be consistent with all the
> >> other ports rather than change os_posix.cpp to adust for PPC/AIX.
> >> JDK-8163900 will most likely be fixed with a simple rename to
> >> os::caller_frame() given that this is what all the users of this API
> >> actually want. I can't think of any reason why the stack trace would
> >> want to include the frame of whoever calls os::current_frame(), so the
> >> rename seems like the most practical fix.
> >>>
> >>> "8153743: AllocateHeap() and ReallocateHeap() should use
> ALWAYSINLINE
> >> macro"
> >>> is not properly implemented on Aix. The 'inline' keyword is missing in the
> >>> macro on aix.
> >> But you also changed the test to expect ALWAYSINLINE not to work with
> >> PPC slowdebug builds? When is adding "inline" helping? Is it needed for
> >> AIX optimized builds?
> >>>
> >>> Also, on Aix ALWAYSINLINE has no effect in the slowdebug build. So the
> >>> check in the test whether AllocateHeap is inlined must be skipped as on
> >>> other platforms.
> >> See the above comment. Otherwise, the changes to the check for AIX in
> >> the test are fine.
> >>
> >> BTW, the following CR might also be of interest to you as it relates to
> >> why the test needs this extra check:
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8163899
> >> JDK-8163899 NMT frame skipping code is fragile
> >>
> >> cheers,
> >>
> >> Chris
> >>>
> >>> Best regards,
> >>>    Goetz.
> >>
> >