Hi Fredrik,
On 18/10/2013 11:41 PM, Fredrik Arvidsson wrote:
Hi
Please help me with this tiny review:
Webrev:
http://cr.openjdk.java.net/~farvidsson/8026808/webrev.00/index.html
<http://cr.openjdk.java.net/%7Efarvidsson/8026808/webrev.00/index.html>
Bug: https://bugs.openjdk.java.net/browse/JDK-8026808
About this bug:
This bug was revealed by another test-fix:
https://bugs.openjdk.java.net/browse/JDK-8025638. The JDKToolLauncher
have probably never worked in Solaris 64 environments.
I don't see the connection to 64-bit from the fix but I suspect it may
be a confusion between launcher args (like -d64) and VM args (like
-XX:+foo). AFAICS the tools will accept both -d64 and -J-d64.
Your change to vmArgs is wrong. It explicitly states that it adds -J to
the arg and you have removed that. When launching a tool (javac, javap,
etc) you have to pass VM args via -J - and that is what this method was
supposed to do. AFAICS based on the usage example given in the code:
40 * JDKToolLauncher jmap = JDKToolLauncher.create("jmap")
41 * .addVMArg("-XX:+PrintGC");
42 *
.addVMArg("-XX:+PrintGCDetails")
43 * .addToolArg("-heap")
44 * .addToolArg(pid);
45 * ProcessBuilder pb = new ProcessBuilder(jmap.getCommand());
the existing code was correct.
Based on the jtreg log in the bug report:
Command line:
[/export/local/aurora/sandbox/sca/vmsqe/jdk/nightly/fastdebug/rt_baseline/solaris-amd64/bin/java
-d64 -Xmx1g JMapHProfLargeHeapProc 22528 ]
Extracted pid: 18298
jmap command:
[/export/local/aurora/sandbox/sca/vmsqe/jdk/nightly/fastdebug/rt_baseline/solaris-amd64/bin/jmap,
-d64, -dump:format=b,file=18298-heap.hprof, 18298]
it would seem to me that the problem is the presence of commas in the
jmap command (compare with the java command). Not that I can see where
they come from. I can't find this test in our repo - where is it?
David
-----
JTReg tests run in JPRT:
http://sthjprt.se.oracle.com/archives/2013/10/2013-10-18-094943.farvidss.hotspot/
verified successful.
Cheers
/F
