RFR: 8139564: Convert TraceDefaultMethods to Unified Logging (original) (raw)

harold seigel harold.seigel at oracle.com
Mon Oct 19 12:48:58 UTC 2015

Hi Rachel,

I agree that the ResourceMark at line 940 should be left in.

Thanks, Harold

On 10/16/2015 5:32 PM, Rachel Protacio wrote:

Thanks for the review.

On 10/16/2015 12:13 PM, harold seigel wrote: Hi Rachael,

The code looks good. One comment about this FOR loop in defaultMethods.cpp: 686 for (int i = 0; i < slots->length(); ++i) { 687 ResourceMark rm; 688 outputStream* logstream = LogHandle(defaultmethods)::debugstream(); 689 streamIndentor si(logstream); 690 logstream->indent(); 691 slots->at(i)->printon(logstream); 692 logstream->cr(); 693 } Can you move lines 687-689 above the loop so that they only get executed once per FOR loop, instead of every iteration? Good idea. Also, you may not need the ResourceMark at line 831 because there is one at line 809. 830 if (logisenabled(Debug, defaultmethods)) { 831 ResourceMark rm; 832 outputStream* logstream = LogHandle(defaultmethods)::debugstream(); 833 streamIndentor si(logstream, 2); 834 logstream->indent().print("Looking for default methods for slot "); 835 slot->printon(logstream); 836 logstream->cr(); Also done. Finally, is the ResourceMark needed at line 940? Well, every LogHandle stream needs a ResourceMark, which is why I included it there and in line 831. As you pointed out, the latter one has a ResourceMark in the code above it. Although technically the one at 940 is currently only called within the scope of the 809 ResourceMark, since it is in a separate function, it's probably safer to leave it. What do you think? Thanks, Rachel Thanks, Harold On 10/15/2015 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->printcr("Slots that need filling:"); 687 streamIndentor si(tty); 688 for (int i = 0; i < slots->length(); ++i) { 689 tty->indent(); 690 slots->at(i)->printon(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::printon(), 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

More information about the hotspot-runtime-dev mailing list