RFR: 8143157: Convert TraceVMOperation to Unified Logging (original) (raw)

David Holmes david.holmes at oracle.com
Wed Nov 18 09:22:46 UTC 2015

Hi Rachel,

On 18/11/2015 5:50 AM, Rachel Protacio wrote:

Hi,

Please review the following small logging enhancement. Summary: The former -XX:+TraceVMOperation flag is updated to the unified logging framework and is now replaced with -Xlog:vmoperation in product mode. Open webrev: http://cr.openjdk.java.net/~rprotacio/8143157/ Bug: https://bugs.openjdk.java.net/browse/JDK-8143157 Testing: Passes jtreg, JPRT, RBT quick tests, and refworkload performance tests.

Meta-question: the logging framework is safe to be called when at a safepoint isn't it?

src/share/vm/runtime/vm_operations.cpp

void VM_Operation::evaluate() { ResourceMark rm; ! outputStream* debugstream = LogHandle(vmoperation)::debug_stream(); ! if (log_is_enabled(Debug, vmoperation)) { ! debugstream->print("["); ! print_on_error(debugstream); ! debugstream->cr(); } doit(); ! if (log_is_enabled(Debug, vmoperation)) { ! print_on_error(debugstream); ! debugstream->print_cr("]"); } }

Why are you calling print_on_error twice?

Why is the only logging level for this tag the "debug" level? I think I may be missing part of the way UL works here - can you enable logging both by explicit tag (ie vmoperation) and the level (ie debug)?

And I know I sound like a broken record but I'm concerned about the overhead of the logging actions when it is not enabled ie:

outputStream* debugstream = LogHandle(vmoperation)::debug_stream();

always gets executed - and we evaluate is_enabled twice.

test/runtime/logging/VMOperationTestMain.java

Can you add a comment explaining what the logic is attempting to do please. I'm curious how many times the loop executes before you get a safepoint-based GC (and how it varies for different GC's).

Style nit: while( -> while (

A compatability request has been accepted with regard to this change.

I'll record my objections again to the conversion of develop flags to product.

Thanks, David

Thank you very much, Rachel

More information about the hotspot-runtime-dev mailing list