On Fri, 18 Jun 2021 06:17:24 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Denghui Dong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> polish > > src/hotspot/share/runtime/os.cpp line 402: > >> 400: // the output stream (e.g. tty->flush()) after output. See >> 4803766. >> 401: // Each module also prints an extra carriage return after its >> output. >> 402: VM_PrintThreads op(tty, PrintConcurrentLocks, false, true); > > Please comment what the false/true arguments mean e.g. > > ... false /* no extended info */, true /* print JNI handle info */); fixed. > src/hotspot/share/runtime/vmOperation.hpp line 112: > >> 110: template(JFROldObject) \ >> 111: template(JvmtiPostObjectFree) \ >> 112: template(ExtendedPrintThreads) > > This change is not needed now. fixed. > src/hotspot/share/runtime/vmOperations.hpp line 146: > >> 144: bool _print_concurrent_locks; >> 145: bool _print_extended_info; >> 146: bool _print_jni; > > Please call this _print_jni_handle_info and update parameter names > accordingly. > > Though it seems we never pass false here and so always print the JNI handle > info, in which case we can get rid of the field and the parameter. fixed. > src/hotspot/share/runtime/vmOperations.hpp line 150: > >> 148: VM_PrintThreads() >> 149: : _out(tty), _print_concurrent_locks(PrintConcurrentLocks), >> _print_extended_info(false), _print_jni(false) >> 150: {} > > This no-arg constructor is unused now. the no-arg constructor is used by JVM_DumpAllStacks now. > src/hotspot/share/runtime/vmOperations.hpp line 151: > >> 149: : _out(tty), _print_concurrent_locks(PrintConcurrentLocks), >> _print_extended_info(false), _print_jni(false) >> 150: {} >> 151: VM_PrintThreads(outputStream* out, bool print_concurrent_locks, bool >> print_extended_info, bool print_jni = false) > > As far as I can see you don't rely on the default parameter value so it is > not needed. fixed > src/hotspot/share/services/attachListener.cpp line 187: > >> 185: >> 186: // thread stacks and JNI global handles >> 187: VM_PrintThreads op1(out, print_concurrent_locks, print_extended_info, >> true); > > Please comment what the true argument means. fixed > src/hotspot/share/services/diagnosticCommand.cpp line 538: > >> 536: void ThreadDumpDCmd::execute(DCmdSource source, TRAPS) { >> 537: // thread stacks and JNI global handles >> 538: VM_PrintThreads op1(output(), _locks.value(), _extended.value(), >> true); > > Please comment what the true argument means. fixed ------------- PR: https://git.openjdk.java.net/jdk/pull/4504