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

[Yekaterina Kantserova]

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20150428/681d95f4/attachment.html>