> On 24 apr 2015, at 11:34, Yekaterina Kantserova > <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/ > webrev jdk: http://cr.openjdk.java.net/~ykantser/8059047.jdk/webrev.01/ > <http://cr.openjdk.java.net/~ykantser/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/~ykantser/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> wrote: >> >>>>> >> >>>>>Hi, >> >>>>> >> >>>>>Could I please have a review of this fix. >> >>>>> >> >>>>>bug:https://bugs.openjdk.java.net/browse/JDK-8059047 >> >>>>>webrev:http://cr.openjdk.java.net/~ykantser/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 >
