Staffan, thanks for your review!
The new webrev can be found here:
http://cr.openjdk.java.net/~ykantser/6545295/webrev.01/
// Katja
On 06/13/2014 01:44 PM, Staffan Larsen wrote:
Katja,
Great to see this simplification! A couple of comments:
- I don’t think you need to launch jhat with -XX:+UsePerfData - no one is
attaching or reading data from the process.
- The static processBuilder field seems to be mis-used. Why not use different
ProcessBuilder objects for the HelloWorld and jhat?
- For dumpFile.deleteOnExit() to be useful you should add it as early as
possible (line 48 perhaps?). If it is the last line in the test you can just as
well do dumpFile.delete().
Of cause it should be dumpFile.delete()! - dumpFile.deleteOnExit() has
crept in by mistake.
/S
On 13 jun 2014, at 13:25, Yekaterina Kantserova
<[email protected]> wrote:
Hi,
Could I please have a review of this fix.
webrev: http://cr.openjdk.java.net/~ykantser/6545295
bug: https://bugs.openjdk.java.net/browse/JDK-6545295
The test has been re-written using the testlibrary's functionality. It's
verified internally on all basic platforms.
Thanks,
Katja
