RFR: 8295060: Port PrintDeoptimizationDetails to UL [v2]

[Johan Sjölen]

On Tue, 11 Oct 2022 23:53:10 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix review comments
>
> src/hotspot/share/runtime/deoptimization.cpp line 441:
> 
>> 439:   if (trap_scope->rethrow_exception()) {
>> 440: #ifndef PRODUCT
>> 441:     log_debug(deoptimization)("Exception to be rethrown in the interpreter for method %s::%s at bci %d", trap_scope->method()->method_holder()->name()->as_C_string(), trap_scope->method()->name()->as_C_string(), trap_scope->bci());
> 
> While you are here could you break this up into three lines please.

Fixed.

> src/hotspot/share/runtime/deoptimization.cpp line 1472:
> 
>> 1470:         {
>> 1471:           LogMessage(deoptimization) lm;
>> 1472:           if (lm.is_debug()) {
> 
> Should this be trace level to match the fact you also needed Verbose before?

It should be, fixed.

> src/hotspot/share/runtime/vframe.cpp line 681:
> 
>> 679: #ifndef PRODUCT
>> 680: void vframe::print() {
>> 681:   if (WizardMode) print_on(tty);
> 
> You don't need the `WizardMode` guard here and in `print_on`.  UL is supposed replace WizardMode and Verbose so the correct fix would be to elide it from `print_on` and only log using `print_on` when the logging level is logically equivalent to `WizardMode` (as you have done elsewhere).

This sounds OK to me.

> src/hotspot/share/runtime/vframeArray.cpp line 380:
> 
>> 378: #ifndef PRODUCT
>> 379:   log_debug(deoptimization)("Locals size: %d", locals()->size());
>> 380:   auto log_it = [](int i, intptr_t* addr) {
> 
> Again I don't see why this can't just be inline

Fixed

> test/hotspot/gtest/logging/test_logStream.cpp line 158:
> 
>> 156:   EXPECT_TRUE(file_contains_substring(TestLogFileName, "ABCD\n"));
>> 157: }
>> 158: 
> 
> Inadvertent removal?

It's the last line, so it's a spurious newline to me.

> test/hotspot/jtreg/compiler/uncommontrap/TestDeoptOOM.java line 45:
> 
>> 43:  *      -XX:CompileCommand=exclude,compiler.uncommontrap.TestDeoptOOM::m9_1
>> 44:  *      -XX:+UnlockDiagnosticVMOptions
>> 45:  *      -XX:+UseZGC -XX:+LogCompilation -XX:+TraceDeoptimization -XX:+Verbose
> 
> Why isn't this enabling deoptimization logging?

Indeed, I added it as deoptimization=trace as it has verbose turned on.

-------------

PR: https://git.openjdk.org/jdk/pull/10645