RFR(S): 8165315: [ppc] Port "8133749: NMT detail stack trace cleanup"
chris.plummer at oracle.com
Tue Sep 6 16:51:19 UTC 2016
Comments inline below:
On 9/5/16 12:28 AM, Lindenmaier, Goetz wrote:
> 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.
> Please review this change. I also please need a sponsor.
> 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:
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:
JDK-8163899 NMT frame skipping code is fragile
> Best regards,
More information about the hotspot-runtime-dev