RFR: JDK-8163185: jhsdb jstack cannot work with normal mode

[Dmitry Samersoff]

Yasumasa,

I'll sponsor the push.

-Dmitry

On 2016-08-09 04:01, David Holmes wrote:
> On 8/08/2016 11:17 PM, Yasumasa Suenaga wrote:
>> Hi David,
>>
>>> It is a better fix in my opinion but still changes the behaviour of
>>> jhsdb - or is that not an issue? Is jhsdb new with 9?
>>
>> Can I list you as a reviewer?
>> This change is agreed by Dmitry. So I think we can move forward it.
> 
> Yes to both. :)
> 
> Thanks,
> David
> 
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2016/08/05 20:38, David Holmes wrote:
>>> On 5/08/2016 8:55 PM, Yasumasa Suenaga wrote:
>>>> Hi David,
>>>>
>>>> Thank you for your comment.
>>>>
>>>> For not to break current behavior, I think we should not change
>>>> c'tor of
>>>> JStack.
>>>> I uploaded new webrev:
>>>>
>>>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8163185/webrev.01/
>>>>
>>>> David, is it okay?
>>>
>>> It is a better fix in my opinion but still changes the behaviour of
>>> jhsdb - or is that not an issue? Is jhsdb new with 9?
>>>
>>> Thanks,
>>> David
>>>
>>>> Dmitry, could you review again?
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2016/08/05 14:49, David Holmes wrote:
>>>>> On 5/08/2016 12:24 AM, Yasumasa Suenaga wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> This review request is related to [1].
>>>>>>
>>>>>> "jhsdb jstack" should work as normal mode without being added --mixed
>>>>>> option.
>>>>>> However, this command always works as mixed mode.
>>>>>
>>>>> So it seems to me the JStack class has a very poorly designed API and
>>>>> command-line interface. It appears that jstack wants to default to
>>>>> mixedMode and concurrentLocks, but provides no means to disable those
>>>>> defaults. Given there is no way to turn those settings off from the
>>>>> commad-line then the default should be that they are off! Then you
>>>>> would not need current proposed change.
>>>>>
>>>>> That said the proposed fix effectively disables those defaults, which
>>>>> seems to me to be a change in behaviour.
>>>>>
>>>>> I can't see a way to fix this without breaking existing behaviour
>>>>> somewhere. In which case I would change the way the JStack instance is
>>>>> initialized by default ie:
>>>>>
>>>>> public JStack() {
>>>>>     this(true, true);
>>>>> }
>>>>>
>>>>> becomes:
>>>>>
>>>>> public JStack() {
>>>>>     this(false, false);
>>>>> }
>>>>>
>>>>> Or it may be better to handle this on the jhsdb side and change this:
>>>>>
>>>>>        JStack.main(newArgs.toArray(new String[newArgs.size()]));
>>>>> to
>>>>>
>>>>>        JStack js = new jstack(false,false);
>>>>>        js.runWithArgs(newArgs.toArray(new String[newArgs.size()]));
>>>>>
>>>>>
>>>>> Cheers,
>>>>> David
>>>>>
>>>>>> So I uploaded webrev for this issue. Could you review it?
>>>>>>
>>>>>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8163185/webrev.00/
>>>>>>
>>>>>>
>>>>>> I'm jdk 9 reviewer (ysuenaga), but I cannot access JPRT.
>>>>>> So I need a sponsor.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> [1]
>>>>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-August/020087.html
>>>>>>
>>>>>>
>>>>>>
>>>>>>


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.