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

[David Holmes]

Hi Coleen,

On 28/09/2016 6:33 AM, 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

This form has been requested since we started this so that we don't do 
unnecessary activities if logging is not enabled.

Thanks,
David
-----


> 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?
>
> Also since THREAD is available, can you make ResourceMark rm(THREAD);
> take the THREAD argument in both places?
>
> The test looks great.
>
> 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
>