[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(M): JDK-8163011: AArch64: NMT detail stack trace cleanup
From:       Zhengyu Gu <zgu () redhat ! com>
Date:       2017-08-31 12:59:54
Message-ID: 7bd18ec1-e46a-75fc-a760-d4277c19921f () redhat ! com
[Download RAW message or body]

Just keep in mind, that original NMT specification has memory overhead 
restriction (I can not recall the exact number). The memory for extra 
tracking stacks might be prohibitive.

-Zhengyu

On 08/31/2017 07:53 AM, Thomas Stüfe wrote:
> Hi Dmitry,
> 
> On Thu, Aug 31, 2017 at 9:49 AM, dmitry.samersov <
> dmitry.samersoff@bell-sw.com> wrote:
> 
>> Everybody,
>>
>> Please review:
>>
>>      http://cr.openjdk.java.net/~dsamersoff/JDK-8163011/webrev.05/
>>
>>      I would propose different approach to fix JDK-8133740
>> platform-independent way: record all frames but strip unnecessary
>> NMT-internal ones on printing.
>>
>> This approach is safe (we don't depend to compiler inlining and we never
>> strip non-NMT frames) and platform independent, but cost us some extra
>> memory.
>>
>> -Dmitry
>>
>>
>>
> This looks good, I like it. Code is easier to read now and less vulnerable
> to compiler decisions.
> 
> Small nits:
> 
> should_skip_frame() - as this is an implementation detail for the print
> function, could we not just move it into the print function, or at least
> into nativeCallStack.cpp ?
> 
> ---
> 
> for (int index = 0; index < indent; index ++) out->print(" ");
> 
> Could this be done with
> 
> out->print("%*c", indent, ' ');
> 
> instead?
> 
> ---
> 
> The changes in os::_get_previous_fp() for the four platforms affect
> os::get_native_stack() and method handle tracing
> (trace_method_handle_stub). The former may now, for the assert case, print
> more frames. The latter should, in theory, not change behavior at all. But
> this is from reading the code, it might be good to test this.
> 
> Kind Regards, Thomas
> 
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic