Hi Dmitry,

Thank you for your comment.
I uploaded new webrev. Could you review again?

  http://cr.openjdk.java.net/~ysuenaga/JDK-8156033/webrev.01/hotspot/
  http://cr.openjdk.java.net/~ysuenaga/JDK-8156033/webrev.01/jdk/

BasicLauncherTest works fine.


Thanks,

Yasumasa


On 2016/05/06 21:46, Dmitry Samersoff wrote:
Yasumasa,

BasicLauncherTest launches each SA tools with --pid argument only.
Should we add test for generating heapdump in it?

Yes.

Please  make sure that LingeredApp is launched with small heap value
(-Xmx256M) at ll. 103

and add something like

launch("<some string>", "jmap", "--binaryheap",
       "--dumpfile=<filename>");

-Dmitry

On 2016-05-06 15:24, Yasumasa Suenaga wrote:
Hi Dmitry,

Thank you for reviewing!

On 2016/05/06 21:06, Dmitry Samersoff wrote:
Yasumasa,

The changes looks good. I'll sponsor the push.

Thanks!


1. I would prefer to check arguments explicitly for better readability:

if (!requestHeapDump && dumpfile != null) {
    throw IAE("Unexpected argument dumpfile");
}

I'll fix it.


2. Please update

jdk/test/sun/tools/jhsdb/BasicLauncherTest.java

BasicLauncherTest launches each SA tools with --pid argument only.
Should we add test for generating heapdump in it?


Thanks,

Yasumasa


-Dmitry

On 2016-05-04 17:47, Yasumasa Suenaga wrote:
Hi all,

This review request relates to [1].

We cannot change heapdump name (heap.bin) when we genarate heapdump via
jhsdb.

When we use jmap, we can change heapdump name through file option.
I want to set it like a jmap.

I uploaded webrev for this enhancement.
Could you review it?

  http://cr.openjdk.java.net/~ysuenaga/JDK-8156033/webrev.00/

I cannot access JPRT.
So I need a sponsor.


Thanks,

Yasumasa


[1]
http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-April/019510.html






Reply via email to