RFR: 8268857: Merge VM_PrintJNI and VM_PrintThreads and remove the unused field 'is_deadlock' of DeadlockCycle [v5]

[Denghui Dong]

On Fri, 18 Jun 2021 06:17:24 GMT, David Holmes <dholmes at 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