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

[David Holmes]

Hi Goetz,

On 6/09/2016 6:35 PM, Lindenmaier, Goetz wrote:
> Hi David,
>
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-August/020574.html
> as you voted for (2) here, I did this fix this way.  If cleaning up the semantics of
> current_frame, the PPC define should be fixed.

Thats for the memory job, I'd lost the context that this is the missing 
PPC part of what Chris did. But in that RFR Chris wrote:

-linux-ppc: Hard to say for sure since the implementation of
os::current_frame is different than others, but it looks to me like it 
suffers from both JDK-8133749 and JDK-8133740.

so I guess I'm expecting to see fixes in the platform code not the 
shared code for this. ??

Thanks,
David

> Best regards,
>   Goetz.
>
>> -----Original Message-----
>> From: David Holmes [mailto:david.holmes at oracle.com]
>> Sent: Dienstag, 6. September 2016 01:35
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Volker Simonis
>> <volker.simonis at gmail.com>
>> Cc: hotspot-runtime-dev at openjdk.java.net; Chris Plummer
>> <chris.plummer at oracle.com>
>> Subject: Re: RFR(S): 8165315: [ppc] Port "8133749: NMT detail stack trace
>> cleanup"
>>
>> Adding in Chris Plummer specifically ...
>>
>> On 6/09/2016 12:25 AM, Lindenmaier, Goetz wrote:
>>> HI Volker,
>>>
>>> thanks for looking at this.
>>>
>>>> Does it mean that other platforms except ppc64 are always missing the
>> first frame of stack traces in the hs_err
>>> Yes, in some cases:
>>>        frame fr = _context ? os::fetch_frame_from_context(_context)
>>>                            : os::current_frame();
>>>        print_native_stack(st, fr, _thread, buf, sizeof(buf));
>>
>> Chris has been working in this area so I really want to hear his take on
>> this. I really don't like seeing PPC specific code in os_posix.cpp, and
>> I really want to understand why it is needed, especially when the
>> implication is that PPC os::current_frame() is correct and all others
>> seem to be wrong!
>>
>> Thanks,
>> David
>>
>>> Best regards,
>>>   Goetz.
>>>
>>>
>>>> -----Original Message-----
>>>> From: Volker Simonis [mailto:volker.simonis at gmail.com]
>>>> Sent: Montag, 5. September 2016 16:16
>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
>>>> Cc: hotspot-runtime-dev at openjdk.java.net
>>>> Subject: Re: RFR(S): 8165315: [ppc] Port "8133749: NMT detail stack trace
>>>> cleanup"
>>>>
>>>>
>>>>
>>>> On Monday, September 5, 2016, Lindenmaier, Goetz
>>>> <goetz.lindenmaier at sap.com <mailto:goetz.lindenmaier at sap.com> >
>>>> 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.
>>>>
>>>> 	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.bs/>
>>>> 	http://cr.openjdk.java.net/~goetz/wr16/8165315-
>>>> fixStackTrace/01/webrev.hs/
>>>> <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.)
>>>>
>>>>
>>>>
>>>>
>>>> Hi Goetz,
>>>>
>>>> Your change looks good. I just don't understand the difference between
>>>> current_frame() on various platforms. Does it mean that other platforms
>>>> except ppc64 are always missing the first frame of stack traces in the
>> hs_err
>>>> file ?
>>>>
>>>> Thanks,
>>>> Volker
>>>>
>>>>
>>>> 	"8153743: AllocateHeap() and ReallocateHeap() should use
>>>> ALWAYSINLINE macro"
>>>> 	is not properly implemented on Aix. The 'inline' keyword is missing in
>>>> the
>>>> 	macro on aix.
>>>>
>>>> 	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.
>>>>
>>>> 	Best regards,
>>>> 	  Goetz.
>>>>
>>>