RFR: 8298091: Dump native instruction along with nmethod name when using Compiler.codelist

[David Holmes]

On Thu, 2 Feb 2023 02:12:06 GMT, Yi Yang <yyang at openjdk.org> wrote:

> Dump native instruction along with nmethod name when using Compiler.codelist.
> 
> e.g.
> jcmd <pid> Compiler.codelist decode=<method_name>
> 
> Output:
> <compile_id> <compile_level> <state> nmethod_name [addr_begin, code_begin, code_end]
> <native instruction>

I'm not clear exactly how this is intended to be used, and I think there should be usage information to update somewhere.

Further coding suggestions below. I'm stilling mulling this over.

Note there is a very high bar for defining new manageable flags (CSR request required) and I'm not sure this usecase warrants it. Why do we need to force/retry the load of the disassembler library it it previously failed to load? Why do you think it will load successfully on subsequent attempts?

src/hotspot/share/compiler/disassembler.cpp line 773:

> 771:   // Do not try to load multiple times. Failed once -> fails always.
> 772:   // To force retry in debugger: assign _tried_to_load_library=0 or
> 773:   // turn on ForceLoadDisassembler in the fly

"on the fly"

src/hotspot/share/compiler/disassembler.cpp line 774:

> 772:   // To force retry in debugger: assign _tried_to_load_library=0 or
> 773:   // turn on ForceLoadDisassembler in the fly
> 774:   if (!ForceLoadDisassembler && _tried_to_load_library) {

This doesn't seem quite right. If the library has already been loaded and is usable then you don't need to "reload" it even if ForceLoadDisassembler is true. I would expect:

if (_tried_to_load_library) {
  if (_library_usable) {
    return true;
  } else if (!RetryLoadDisassembler) {
    return false;
  }

src/hotspot/share/runtime/globals.hpp line 651:

> 649:                                                                             \
> 650:   product(bool, ForceLoadDisassembler, false, MANAGEABLE,                   \
> 651:           "Always loading hotspot disassembler")                            \

That doesn't quite describe what the flag does. I would suggest calling it `RetryLoadDisassembler` with a description:

Always try to reload the disassembler, even if a previous load failed"

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

Changes requested by dholmes (Reviewer).

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