RFR: 8139564: Convert TraceDefaultMethods to Unified Logging

[harold seigel]

Hi Rachel,

The changes look good.

Thanks, Harold

On 10/19/2015 5:02 PM, Rachel Protacio wrote:
> 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
>>>>>>
>>>>>
>>>>
>>>
>>
>