RFR: JDK-8185003 JMX: Add a version of ThreadMXBean.dumpAllThreads with a maxDepth argument

[Ujwal Vangapally]

Thanks for the review Roger, please see my comments inline.


On 8/3/2017 8:23 PM, Roger Riggs wrote:
> Hi Ujwal,
>
> (Reviewer, but not specifically servicability).
>
> Comments,
>
> java/lang/management/ThreadMXBean.java:
>
> 809: It may be useful to state that the behavior is the same as {@link 
> #dumpAllThreads} except that the depth is limited.
>
> 828:  Do not duplicate specification of the meaning of maxDepth = 
> MAX_VALUE, either put the entire spec
> in the @param or in the description.  Duplication creates an 
> opportunity for disagreement or a maintenance issue when updating.
>
> 833: terminate the {@code maxDepth} before "is negative".
>
> As Erik comments, there should be a space after "," everywhere; both 
> to be consistent with existing style and the style guide.
will make changes to doc as suggested.
>
> sun/management/ThreadImpl.java:
>
> 484: Since the native code is going to check maxDepth, why is it 
> checked here also?
>     -or- remove the check in native
As  maxDepth value should not be negative for dumpAllThreads 
sun/management/ThreadImpl.java: 484: throws IllegalArgumentException.

Check on native side can be removed but having it might help in 
enforcing the convention of using only -1 for dumping all the stack frames
otherwise any negative number can be passed to dump all the frames.
> 490:  This could be simpler to not translate maxDepth to -1,
>      Here and in the native code, passing MAX_INTEGER would dump the 
> entire stack, there is no need to special case it
>
> services/threadService.cpp:
> 565:  checking count >= maxDepth is a sufficient test if to dump all 
> frames, passing MAX_INTEGER is used
>       and it will dump zero should a negative number get this far.
>
currently getThreadInfo method accepts maxDepth argument and it uses the 
translation of maxDepth to "-1"  as it's implementation detail for 
printing all stack frames,
As getThreadInfo also uses services/threadService.cpp: 565: 
(dump_stack_at_safepoint) expecting it to print all stack trace elements 
when maxDepth is given as "-1".
I followed same convention so that it will not affect getThreadInfo 
method with maxDepth argument.
> Thanks, Roger
>

Thanks,
Ujwal
> On 8/3/2017 10:18 AM, Ujwal Vangapally wrote:
>> kindly use the below link for accessing webrev
>>
>> http://cr.openjdk.java.net/~uvangapally/webrev/2017/8185003/webrev.01/
>>
>> previously shared link is no longer accessible hence providing this 
>> new link.
>>
>> Thanks,
>>
>> Ujwal.
>>
>>
>> On 8/3/2017 3:08 PM, Ujwal Vangapally wrote:
>>> Hi,
>>>
>>> kindly review the changes made.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8185003
>>>
>>> webrev: 
>>> http://cr.openjdk.java.net/~uvangapally/webrev/2017/8185003/webrev.00/
>>>
>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8185705
>>>
>>> Thanks,
>>>
>>> Ujwal.
>>>
>>
>