RFR(M): 8059047: Extract parser/validator from jhat for use in tests

[Yekaterina Kantserova]

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.kantserova at oracle.com 
>> <mailto:yekaterina.kantserova at 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.kantserova at oracle.com 
>>> <mailto:yekaterina.kantserova at 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
>>
>