RFR(m): 8150015: Integrate TraceTime with Unified Logging more seamlessly

Coleen Phillimore coleen.phillimore at oracle.com
Thu Mar 17 02:03:32 UTC 2016



On 3/16/16 1:27 AM, David Holmes wrote:
> On 16/03/2016 3:14 PM, David Holmes wrote:
>> Hi Robbin,
>>
>> Thanks for tackling this. :)
>>
>> On 16/03/2016 6:31 AM, Robbin Ehn wrote:
>>> Hi, please review this enhancement.
>>>
>>> This adds support for multiple UL tags in TraceTime.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8150015/
>>> Webrev: http://cr.openjdk.java.net/~rehn/8150015/v1/webrev/
>>
>> Initial query: why do so many .cpp files now include the timerTrace.hpp
>> file yet have no other changes?
>
> Okay this is because they already implicitly include timer.hpp but now 
> have to include the new timerTrace.hpp. Fair enough.

Now I see why also.

>
> Minor nit: src/share/vm/compiler/compileBroker.cpp - the include was 
> not put in in alphabetical order.
>
>> I'm still mulling over the verbosity of the new format. You may recall
>> there was a lot of discussion about this last time round :)
>
> Do we perhaps want to keep the wrapper classes in logTimer.hpp? 
> Additional uses of TraceTime with UL can then either add their own 
> wrappers or use the direct form you have added support for.
>

I think this looks good.  The verbosity is far less than I thought it 
would be and having a macro TRACETIME_LOG is a good indication that 
something strange and macro-y is going on.   Another clever use of 
templates and macros here, but fine at the call sites.

thank you for fixing this.

Coleen


> Thanks,
> David
>
>> Thanks,
>> David
>>
>>> Tested with jprt hotspot and manually tested log output.
>>>
>>> Thanks!
>>>
>>> /Robbin



More information about the hotspot-runtime-dev mailing list