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
