RFR: 8141211: Convert TraceExceptions to Unified Logging

[David Holmes]

Hi Rachel,

On 9/12/2015 1:42 AM, Rachel Protacio wrote:
> Hello,
>
> Please review my conversion of -XX:+TraceExceptions to
> -Xlog:exceptions=info. The existing (product) flag is aliased to the
> logging flag at the info level.

Q: how does use of ttyLocker map into UL? I see an awful lot of 
multi-line logging blocks that are going to be completely messed up 
without locking!

src/share/vm/opto/runtime.cpp

1239     ResourceMark rm;

What requires the introduction of the ResourceMark?

---

src/share/vm/runtime/thread.cpp

2084       if (log_is_enabled(Info, exceptions)) {
2085          ResourceMark rm;

An extra leading space has crept in at +2085

2087         logstream->print("Async. exception installed at runtime 
exit (" INTPTR_FORMAT ")", p2i(this));
2088          if (has_last_Java_frame()) {
2089            frame f = last_frame();
2090           logstream->print(" (pc: " INTPTR_FORMAT " sp: " 
INTPTR_FORMAT " )", p2i(f.pc()), p2i(f.sp()));
2091          }

Indention of these lines is wrong at #2088 and #2089 - need an 
additional space.

2092         logstream->print_cr(" of type: %s", 
InstanceKlass::cast(_pending_async_exception->klass())->external_name());

Why did the InstanceKlass::cast need to be introduced ??

---

test/runtime/CommandLine/TraceExceptionsTest.java

You improved the readability of the source code by breaking the @summary 
over two lines, but IIRC jtreg will write the summary into the test log 
with all the additional spaces you added, as the summary extends until 
the next tag.

---

test/runtime/logging/ExceptionsTest.java

Can you avoid the code duplication please.

Also perhaps you could/should also check there are no logging statements 
present when logging is supposed to be disabled. I'm guessing the other 
logging tests haven't considered this. ??

> If you have any questions on the alias table (in the arguments.cpp and
> .hpp files), Max will chime in as he is the one who implemented that
> portion.

Okay some general questions.

First I would expect that aliased logging options are also marked 
deprecated so that we can eventually make them obsolete and remove them.

Secondly, arguably if someone specifies -XX:-TraceExceptions it should 
disable exception logging - which may have been turned on by -Xlog:all.

Which leads to: how does the position of -XX:+/-TraceExceptions interact 
with the position of -Xlog:xxx on the command-line (or the other 
argument introducing mechanisms) ?

src/share/vm/runtime/arguments.cpp

2743       if(lookup_logging_aliases(option->optionString, 
aliased_logging_option)){

Need space after "if", and before {

Thanks,
David
-----


> Open webrev: http://cr.openjdk.java.net/~rprotacio/8141211/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8141211
>
> Testing: jtreg, JPRT, jck vm tests, refworkload performance tests, rbt
> quick & non-colo tests
>
> Thank you!
> Rachel