RFR: 8139564: Convert TraceDefaultMethods to Unified Logging

[Rachel Protacio]

Thanks, Harold!
Rachel

On 10/20/2015 10:52 AM, harold seigel wrote:
> 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
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>