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
