Dmitry, thanks for undertaking this.
Thumbs up from me!
-JB-
On 17.7.2015 16:29, serguei.spit...@oracle.com wrote:
Dmitry,
Thanks for new webrev!
A couple of comments on
hotspot_webrev/agent/src/share/classes/sun/jvm/hotspot/SAGetopt.java
The following fragment has the same invariant for bo
Serguei,
Thank you for review.
> The following fragment has the same invariant for both branches:
fixed. (webrev updated in-place, diff to webrev.06.old is attached)
> One more suggestion would be to refactor the if (_optreset) { ... }
> with a cal to a new method optReset().
I would prefer to
Dmitry,
Thanks for new webrev!
A couple of comments on
hotspot_webrev/agent/src/share/classes/sun/jvm/hotspot/SAGetopt.java
The following fragment has the same invariant for both branches:
136 ch = carg.charAt(_optopt);
137 }
138 else {
139 ch = ca
Serguei,
Sorry for typeo
new webrev:
http://cr.openjdk.java.net/~dsamersoff/JDK-8059038/webrev.06
-Dmitry
On 2015-07-17 16:28, serguei.spit...@oracle.com wrote:
> On 7/17/15 6:21 AM, Dmitry Samersoff wrote:
>> Serguei,
>>
>> new webrev:
>>
>> http://cr.openjdk.java.net/~dsamersoff/JDK-8059038/
On 7/17/15 6:21 AM, Dmitry Samersoff wrote:
Serguei,
new webrev:
http://cr.openjdk.java.net/~dsamersoff/JDK-8059038/webrev.05
The webrev is the same.
I do not see the changes you claim below.
Could you, please, generate a webrev with another version number?
Thanks,
Serguei
diff to webrev
Serguei,
new webrev:
http://cr.openjdk.java.net/~dsamersoff/JDK-8059038/webrev.05
diff to webrev.05.old attached
please see also below.
On 2015-07-17 13:46, serguei.spit...@oracle.com wrote:
> Dmitry,
>
> Thanks for the diff, it helps!
>
> hotspot_webrev/agent/src/share/classes/sun/jvm/hotsp
Dmitry,
Thanks for the diff, it helps!
hotspot_webrev/agent/src/share/classes/sun/jvm/hotspot/SAGetopt.java
Uninitialized local definition:
105 char ch;
Unneded second initialization at 111:
104 String carg = _argv[_optind];
111 carg = _argv[_optind];
It is no
Serguei,
Previous webrev is:
http://cr.openjdk.java.net/~dsamersoff/JDK-8059038/webrev.05.old
Latest webrev is:
http://cr.openjdk.java.net/~dsamersoff/JDK-8059038/webrev.05
Diff between webrev.05.old and webrev.05 attached
-Dmitry
On 2015-07-17 01:00, serguei.spit...@oracle.com wrote:
> Hi Dmi
Hi Dmitry,
I do not see any changes.
Could you please, generate .06 version ?
In such a case, it will be much easier to compare the code.
Thanks,
Serguei
On 7/16/15 8:23 AM, Dmitry Samersoff wrote:
Serguei,
http://cr.openjdk.java.net/~dsamersoff/JDK-8059038/webrev.05
Webrev updated in-place
Serguei,
http://cr.openjdk.java.net/~dsamersoff/JDK-8059038/webrev.05
Webrev updated in-place (press shift-reload).
Code changes at ll.119.
Added more comments to other places.
-Dmitry
On 2015-06-27 03:15, serguei.spit...@oracle.com wrote:
> Hi Dmitry,
>
> Thank you for the update!
> The SAL
Hi Dmitry,
Thank you for the update!
The SALauncher.java changes are really nice.
I have just couple of small comments.
agent/src/share/classes/sun/jvm/hotspot/SALauncher.java
343 // Run tmtools
344 if (args[0].equals("jstack")) {
345 runJSTACK(oldArgs);
Why the
Serguei,
Thank you for the review.
New webrev is here:
http://cr.openjdk.java.net/~dsamersoff/JDK-8059038/webrev.05/
I didn't change naming convention in SAGetoptTest.java and keep a_opt,
b_opt etc as it gives better readability. Other concerns are addressed.
BasicLauncherTest changed to use
Hi Dmitry,
Some quick minor comments.
hotspot_webrev/agent/src/share/classes/sun/jvm/hotspot/SAGetopt.java
Formatting is wrong:
57 if (_optind >_argv.length) {
71 String[] ca = carg.split("=",2);
80 if (los.contains(ca[0]+"=")) {
Need to use camel case for java me
Hi Everybody,
Please review the changes:
http://cr.openjdk.java.net/~dsamersoff/JDK-8059038/webrev.04/
I'm about to file CCC request for it but would like to get internal
feedback before that.
This fix is introducing native launcher jhsdb for serviceability agent.
jhsdb
will launch command li
14 matches
Mail list logo
