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?
At least, until April 2016, "jhsdb jstack" worked as normal mode (and not
concurrent lock) mode by default.
jhsdb jstack --help shows as below:
-------------
--locks to print java.util.concurrent locks
--mixed to print both java and native frames (mixed mode)
--exe executable image name
--core path to coredump
--pid pid of process to attach
-------------
Thus I think --locks and --mixed should be false by default.
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
