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

Robbin Ehn robbin.ehn at oracle.com
Thu Mar 17 10:42:04 UTC 2016


Hi Coleen, thanks for reviewing.

On 03/17/2016 03:03 AM, Coleen Phillimore wrote:
>
>
> 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.
>

Ok, I'll push as is, e.g. without logTimer.hpp

Thanks !

/Robbin

> 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