RFR (S): 8004924: NPG: jmap -heap output should contain ClassMetaspaceSize value

[Mikael Gerdin]

Erik,

On 2013-02-26 16:47, Erik Helin wrote:
> Mikael,
>
> thanks for reviewing!
>
> On 02/26/2013 04:19 PM, Mikael Gerdin wrote:
>> Erik,
>>
>> On 2013-02-21 12:41, Erik Helin wrote:
>>> Hi,
>>>
>>> based on internal feedback, I've updated the test. Since the test
>>> spawned the "jmap -heap" process and connected the jmap process to
>>> itself, there was a race condition: if the "jmap -heap" process produced
>>> a lot of output, then the JVM process would not be able to consume this
>>> output because it has been stopped by jmap. At the same time, jmap would
>>> wait for the JVM to consume the output from jmap.
>>>
>>> The test now pipes the output from "jmap -heap" to a fike and then later
>>> analyzes the file.
>>
>> Overall this approach seems much safer than having a java thread copy
>> the contents. However in this case you do not drain the stderr from the
>> process you created so in some cases, for example if the SA is broken
>> then it will throw up a long exception stacktrace on stderr.
>>
>> Please either use ProcessBuilder.redirectErrorStream to pipe all output
>> to one file (and rename it to remove stdout from its name)
>> or use redirectError and pipe that output to a separate file.
>>
>> Thanks
>> /Mikael
>
> Agree, I now use redirectError to pipe stderr to another file.
>
> Please see new webrev at:
> http://cr.openjdk.java.net/~ehelin/8004924/webrev.02/
>

This looks good, ship it!

/M

> Thanks,
> Erik
>
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~ehelin/8004924/webrev.01/
>>>
>>> Thanks,
>>> Erik
>>>
>>> On 02/20/2013 09:50 PM, Erik Helin wrote:
>>>> Hi all,
>>>>
>>>> this change the flag ClassMetapaceSize to the output of jmap -heap. I
>>>> have also added a test that checks the ouput (the test uses the new
>>>> process builder framework).
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~ehelin/8004924/webrev.00/
>>>>
>>>> Bug:
>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8004924
>>>>
>>>> Testing:
>>>> JPRT
>>>>
>>>> Thanks,
>>>> Erik
>>>
>