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

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8139564: Convert TraceDefaultMethods to Unified Logging
From:       Rachel Protacio <rachel.protacio () oracle ! com>
Date:       2015-10-19 21:02:12
Message-ID: 56255A54.7030307 () oracle ! com
[Download RAW message or body]

Please see updated webrev 
http://cr.openjdk.java.net/~rprotacio/8139564.02/ with the following 
changes:
- repositioned and deleted ResourceMark's, as per Harold's suggestions
- fixed copyright year in test file
- moved update_position() line in ostream.cpp because it was breaking 
indentation in defaultmethods logging

Marcus, see reply inline.

On 10/19/2015 10:35 AM, Marcus Larsson wrote:
> Hi,
>
>
> On 2015-10-16 18:21, Coleen Phillimore wrote:
>> Hi,
>>
>> I added the serviceability group so they can comment on this as 
>> well.   I think having logging in the PRODUCT build is requested so 
>> that we can more easily debug customer problems.   That said, we will 
>> not enable logging in product if we see any performance problem.  
>> Also for some options it's possible that these are strictly internal 
>> debugging options and in that case we'll either remove them if 
>> they're no longer useful, or make them Develop level options.
>
> This seems like a good approach to me. The develop level was added to 
> accommodate internal or performance sensitive logging that shouldn't 
> be included in the product.
>>
>> Printing default methods seems to be something that might be 
>> borderline in the second case, but we've decided to make it product 
>> level logging.   We could change our minds about this though, so your 
>> comments are welcome.
>
> If it's borderline to internal wouldn't it be more fitting to use 
> trace level for this logging?
Since it's not a question of verbosity but of audience, we'll leave it 
as it is.

Thanks,
Rachel
>
> Thanks,
> Marcus
>
>>
>> Thanks,
>> Coleen
>>
>> On 10/15/15 6:33 PM, Ioi Lam wrote:
>>>
>>>
>>> On 10/15/15 10:51 AM, Rachel Protacio wrote:
>>>> Hi, Ioi,
>>>>
>>>> Thanks for the comments. While all valid points, the decision by 
>>>> the serviceability team with regards to the logging framework as a 
>>>> whole is to move all the output to product mode. Because of this, I 
>>>> ran performance tests to make sure that the newly-introduced 
>>>> product code will not slow it down. So yes, all the "#ifndef 
>>>> PRODUCT" sections that are necessary for this logging have been 
>>>> liberated to product mode.
>>>>
>>> Thanks Rachel. This makes sense.
>>>
>>> - Ioi
>>>
>>>> Also, I realized I did not remove the TraceDefaultMethods flag from 
>>>> globals.hpp, so here is the link to the updated webrev: 
>>>> http://cr.openjdk.java.net/~rprotacio/8139564.01/
>>>> Which builds appropriately. The change now encompasses all the 
>>>> references to TraceDefaultMethods. A compatibility request has been 
>>>> accepted with regards to this change.
>>>>
>>>> Thanks,
>>>> Rachel
>>>>
>>>> On 10/14/2015 11:58 PM, Ioi Lam wrote:
>>>>> Hi Rachel,
>>>>>
>>>>> Before your changes, this block of code  would be excluded from 
>>>>> product builds:
>>>>>
>>>>>  684 #ifndef PRODUCT
>>>>>  685   if (TraceDefaultMethods) {
>>>>>  686     tty->print_cr("Slots that need filling:");
>>>>>  687     streamIndentor si(tty);
>>>>>  688     for (int i = 0; i < slots->length(); ++i) {
>>>>>  689       tty->indent();
>>>>>  690       slots->at(i)->print_on(tty);
>>>>>  691       tty->cr();
>>>>>  692     }
>>>>>  693   }
>>>>>  694 #endif // ndef PRODUCT
>>>>>
>>>>> but after your change, it will be included in product builds. This 
>>>>> means product builds will have more verbose output than before. It 
>>>>> also means that the product builds will get bigger (because some 
>>>>> printing code, such as EmptyVtableSlot::print_on(), would need to 
>>>>> be enabled for product builds as well).
>>>>>
>>>>> I am not very familiar with UL so maybe this is an FAQ ... while 
>>>>> doing the UL conversion, should we add all the old "ifndef 
>>>>> PRODUCT" logs into the product build?
>>>>>
>>>>> Thanks
>>>>> - Ioi
>>>>>
>>>>> On 10/14/15 7:10 PM, Rachel Protacio wrote:
>>>>>> Hello! Please take a look at my enhancement, the first of the 
>>>>>> runtime logging flags to be converted.
>>>>>>
>>>>>> Summary: The former -XX:+TraceDefaultMethods flag is updated to 
>>>>>> the unified logging framework and is now replaced with 
>>>>>> -Xlog:defaultmethods.
>>>>>>
>>>>>> open webrev: http://cr.openjdk.java.net/~rprotacio/8139564/
>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8139564
>>>>>> testing: Passes JPRT, RBT, and RefWorkload performance testing.
>>>>>>
>>>>>> Thank you,
>>>>>> Rachel
>>>>>
>>>>
>>>
>>
>

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