RFR(S): 8160064: StackWalker implementation added logging option without using UL

[Rachel Protacio]

Hi,


On 9/27/2016 4:33 PM, Coleen Phillimore wrote:
>
> (including Mandy, Brent and Daniel)
>
> Rachel,
>
> This looks good.  I have a couple of minor comments
>
> http://cr.openjdk.java.net/~rprotacio/8160064/src/share/vm/prims/stackwalk.cpp.udiff.html 
>
>
> + if (log_is_enabled(Trace, stackwalk)) {
> + ResourceMark rm;
> + outputStream* st = Log(stackwalk)::trace_stream();
> + st->print(" hidden method: ");
> + method->print_short_name(st);
> + st->cr();
>          }
>
> Do you have to ask if the log is enabled here and the other place? (I 
> added it in this snippet).   I think it's tested implicitly when it 
> creates the trace_stream() but since you have a {} block anyway, it 
> seems clearer to test it.   Does method->print_stream(st) do nothing 
> if the logging isn't enabled?
You're right, even though the code I wrote won't print when logging is 
not enabled, it still executes unnecessary code. I'll put log_is_enabled 
around them.
>
> Also since THREAD is available, can you make ResourceMark rm(THREAD); 
> take the THREAD argument in both places?
Yes, will do.
>
> The test looks great.
Thanks!
Rachel
>
> Thanks,
> Coleen
>
> On 9/27/16 3:47 PM, Rachel Protacio wrote:
>> Hi,
>>
>> Please review this small fix correcting the StackWalker print output 
>> mechanism by moving it to Unified Logging. Includes a new jtreg test, 
>> and passes JPRT.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8160064
>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8160064/
>>
>> Thank you!
>> Rachel
>