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
