Staffan,
Thanks for the review!
// Katja
On 04/28/2015 02:05 PM, Staffan Larsen wrote:
Looks good!
Thanks,
/Staffan
On 24 apr 2015, at 16:17, Yekaterina Kantserova
<yekaterina.kantser...@oracle.com
<mailto:yekaterina.kantser...@oracle.com>> wrote:
All suggestions have been implemented. Please find new webrevs here:
webrev root: http://cr.openjdk.java.net/~ykantser/8059047/webrev.02
webrev jdk: http://cr.openjdk.java.net/~ykantser/8059047.jdk/webrev.02
webrev hotspot:
http://cr.openjdk.java.net/~ykantser/8059047.hotspot/webrev.01
// Katja
On 04/24/2015 12:10 PM, Staffan Larsen wrote:
On 24 apr 2015, at 11:34, Yekaterina Kantserova
<yekaterina.kantser...@oracle.com
<mailto:yekaterina.kantser...@oracle.com>> wrote:
Hi,
Here comes the updated version.
bug: https://bugs.openjdk.java.net/browse/JDK-8059047
webrev root:
http://cr.openjdk.java.net/~ykantser/8059047/webrev.01/
<http://cr.openjdk.java.net/%7Eykantser/8059047/webrev.01/>
webrev jdk:
http://cr.openjdk.java.net/~ykantser/8059047.jdk/webrev.01/
<http://cr.openjdk.java.net/%7Eykantser/8059047.jdk/webrev.01/>
test/com/sun/management/HotSpotDiagnosticMXBean/DumpHeap.java:
In main() I think you should change the deleteOnExit() to just
delete(). There is no reason to wait with it.
Perhaps we should also verify that the file does not already exists
before we try to write to it. If it exists, we can delete it.
webrev hotspot:
http://cr.openjdk.java.net/~ykantser/8059047.hotspot/webrev.00/
<http://cr.openjdk.java.net/%7Eykantser/8059047.hotspot/webrev.00/>
test/serviceability/dcmd/gc/HeapDumpTest.java:
Same thing: call delete() instead of deleteOnExit().
One comment about changes in hotspot part. The refactored version
of serviceability/dcmd/gc/HeapDumpTest.java doesn't contain check:
70 /*
71 * Some hprof dumps of all objects contain
constantPoolOop references that cannot be resolved, so we ignore
72 * failures about resolving constantPoolOop fields
using a negative lookahead
73 */
74 output.shouldNotMatch(".*WARNING(?!.*Failed to
resolve object.*constantPoolOop.*).*");
It depends on that the current version of jdk.test.lib.hprof parser
simply write down warnings to stdout. As a result the test needs to
invent own logic to parse it.
I suggest instead to improve jdk.test.lib.hprof parser as a
separate RFE. The parser will collect such information and provide
a new method for getting it, e.g.
jdk.test.lib.hprof.model.Snapshot.getWarnings(). The
serviceability/dcmd/gc/HeapDumpTest.java will be changed
accordingly when RFE is implemented.
Sounds right. A quick, hacky solution is to redirect System.out to a
stream or file while running the parser…
/Staffan
Thanks,
Katja
On 04/22/2015 03:09 PM, Staffan Larsen wrote:
On 22 apr 2015, at 11:17, Yekaterina
Kantserova<yekaterina.kantser...@oracle.com
<mailto:yekaterina.kantser...@oracle.com>> wrote:
>>>>>
>>>>>Hi,
>>>>>
>>>>>Could I please have a review of this fix.
>>>>>
>>>>>bug:https://bugs.openjdk.java.net/browse/JDK-8059047
<http://bugs.openjdk.java.net/browse/JDK-8059047>
>>>>>webrev:http://cr.openjdk.java.net/~ykantser/8059047/webrev.00/
<http://cr.openjdk.java.net/%7Eykantser/8059047/webrev.00/>
>>>>>
>>>>>This fix is a part of JEP 241: Remove the jhat Tool
(https://bugs.openjdk.java.net/browse/JDK-8059039). I suggest to
put parser/validator into common test library since the
functionality can be useful not only for SVC tools tests but even
for some future GC tests.
>>>>>
>>>>>The old jhat packages have been moved as follows:
>>>>>com.sun.tools.hat.internal.model -> jdk.test.lib.hprof.model
>>>>>com.sun.tools.hat.internal.parser -> jdk.test.lib.hprof.parser
>>>>>com.sun.tools.hat.internal.util -> jdk.test.lib.hprof.util
>>>>>
>>>>>The source has not been changed except Copyrights year.
>>>>>
>>>>>Thanks,
>>>>>Katja
