RFR: JDK-8194762: JTReg failure of "runtime/NMT/PrintNMTStatistics.java"

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: JDK-8194762: JTReg failure of "runtime/NMT/PrintNMTStatistics.java"

stewartd.qdt
Please review this small webrev [1] that removes the search for the word "error" in stdout for the PrintNMTStatistics during the NativeMemoryTracking=details sub-test of the JTReg test.

I have found that on my test machine (AArch64, Linux) when the detailed list is printed there is a module called "LogOutputList" that has as one of the listed functions "_dl_catch_error". Since the HTReg test simply searches for the word "error" in stdout, this test fails. However, there really isn't a failure to behave correctly and all the details are printed correctly. It also returns 0 on exit, as it should. The full log is attached to the bug report.

I have created this patch in the belief that the test is being overly restrictive in searching for "error" In stdout and that printing out "_dl_catch_error" is correct behavior and shouldn't cause the test to fail.

Please let me know if this is a bad approach and I'll be happy to change as required.

Thanks,
Daniel Stewart

[1] webrev: http://cr.openjdk.java.net/~dstewart/8194762/webrev.00/
[2] CR: https://bugs.openjdk.java.net/browse/JDK-8194762

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8194762: JTReg failure of "runtime/NMT/PrintNMTStatistics.java"

David Holmes
Hi Daniel,

I think we will need Zhengyu to chime in and explain what NMT related
errors this was intending to catch. The simple search for "error" is
obviously too coarse when you may have various logging output enabled.
Even this:

   64     output_summary.shouldNotContain("error");

may fail if there is additional logging output.

Thanks,
David

On 11/01/2018 1:28 AM, stewartd.qdt wrote:

> Please review this small webrev [1] that removes the search for the word "error" in stdout for the PrintNMTStatistics during the NativeMemoryTracking=details sub-test of the JTReg test.
>
> I have found that on my test machine (AArch64, Linux) when the detailed list is printed there is a module called "LogOutputList" that has as one of the listed functions "_dl_catch_error". Since the HTReg test simply searches for the word "error" in stdout, this test fails. However, there really isn't a failure to behave correctly and all the details are printed correctly. It also returns 0 on exit, as it should. The full log is attached to the bug report.
>
> I have created this patch in the belief that the test is being overly restrictive in searching for "error" In stdout and that printing out "_dl_catch_error" is correct behavior and shouldn't cause the test to fail.
>
> Please let me know if this is a bad approach and I'll be happy to change as required.
>
> Thanks,
> Daniel Stewart
>
> [1] webrev: http://cr.openjdk.java.net/~dstewart/8194762/webrev.00/
> [2] CR: https://bugs.openjdk.java.net/browse/JDK-8194762
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8194762: JTReg failure of "runtime/NMT/PrintNMTStatistics.java"

Zhengyu Gu-2
The fix looks fine to me.

On 01/10/2018 05:26 PM, David Holmes wrote:
> Hi Daniel,
>
> I think we will need Zhengyu to chime in and explain what NMT related
> errors this was intending to catch. The simple search for "error" is
> obviously too coarse when you may have various logging output enabled.
> Even this:
>
>    64     output_summary.shouldNotContain("error");

I don't think NMT emits any error messages, so this line does not make
sense.

Thanks,

-Zhengyu


>
> may fail if there is additional logging output.
>
> Thanks,
> David
>
> On 11/01/2018 1:28 AM, stewartd.qdt wrote:
>> Please review this small webrev [1] that removes the search for the
>> word "error" in stdout for the PrintNMTStatistics during the
>> NativeMemoryTracking=details sub-test of the JTReg test.
>>
>> I have found that on my test machine (AArch64, Linux) when the
>> detailed list is printed there is a module called "LogOutputList" that
>> has as one of the listed functions "_dl_catch_error". Since the HTReg
>> test simply searches for the word "error" in stdout, this test fails.
>> However, there really isn't a failure to behave correctly and all the
>> details are printed correctly. It also returns 0 on exit, as it
>> should. The full log is attached to the bug report.
>>
>> I have created this patch in the belief that the test is being overly
>> restrictive in searching for "error" In stdout and that printing out
>> "_dl_catch_error" is correct behavior and shouldn't cause the test to
>> fail.
>>
>> Please let me know if this is a bad approach and I'll be happy to
>> change as required.
>>
>> Thanks,
>> Daniel Stewart
>>
>> [1] webrev: http://cr.openjdk.java.net/~dstewart/8194762/webrev.00/
>> [2] CR: https://bugs.openjdk.java.net/browse/JDK-8194762
>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8194762: JTReg failure of "runtime/NMT/PrintNMTStatistics.java"

David Holmes
Okay I'm happy that the test will not accidentally pass if unexpected
errors occurs. As the test just does -version then the only errors would
be initialization errors that result in a non-zero return code. As the
test checks for expected output from NMT and that output is produced as
the VM shuts down, then it seems very unlikely for an unrelated error to
occur but still see the NMT output and get a zero return code.

So both occurrences of:

output_detail.shouldNotContain("error");

can be removed.

Thanks,
David

On 11/01/2018 12:19 PM, Zhengyu Gu wrote:

> The fix looks fine to me.
>
> On 01/10/2018 05:26 PM, David Holmes wrote:
>> Hi Daniel,
>>
>> I think we will need Zhengyu to chime in and explain what NMT related
>> errors this was intending to catch. The simple search for "error" is
>> obviously too coarse when you may have various logging output enabled.
>> Even this:
>>
>>    64     output_summary.shouldNotContain("error");
>
> I don't think NMT emits any error messages, so this line does not make
> sense.
>
> Thanks,
>
> -Zhengyu
>
>
>>
>> may fail if there is additional logging output.
>>
>> Thanks,
>> David
>>
>> On 11/01/2018 1:28 AM, stewartd.qdt wrote:
>>> Please review this small webrev [1] that removes the search for the
>>> word "error" in stdout for the PrintNMTStatistics during the
>>> NativeMemoryTracking=details sub-test of the JTReg test.
>>>
>>> I have found that on my test machine (AArch64, Linux) when the
>>> detailed list is printed there is a module called "LogOutputList"
>>> that has as one of the listed functions "_dl_catch_error". Since the
>>> HTReg test simply searches for the word "error" in stdout, this test
>>> fails. However, there really isn't a failure to behave correctly and
>>> all the details are printed correctly. It also returns 0 on exit, as
>>> it should. The full log is attached to the bug report.
>>>
>>> I have created this patch in the belief that the test is being overly
>>> restrictive in searching for "error" In stdout and that printing out
>>> "_dl_catch_error" is correct behavior and shouldn't cause the test to
>>> fail.
>>>
>>> Please let me know if this is a bad approach and I'll be happy to
>>> change as required.
>>>
>>> Thanks,
>>> Daniel Stewart
>>>
>>> [1] webrev: http://cr.openjdk.java.net/~dstewart/8194762/webrev.00/
>>> [2] CR: https://bugs.openjdk.java.net/browse/JDK-8194762
>>>
Reply | Threaded
Open this post in threaded view
|

RE: RFR: JDK-8194762: JTReg failure of "runtime/NMT/PrintNMTStatistics.java"

stewartd.qdt
Thanks, David.

Please see the modified webrev: http://cr.openjdk.java.net/~dstewart/8194762/webrev.01/

Regards,
Daniel

-----Original Message-----
From: David Holmes [mailto:[hidden email]]
Sent: Wednesday, January 10, 2018 9:36 PM
To: Zhengyu Gu <[hidden email]>; stewartd.qdt <[hidden email]>; [hidden email]
Subject: Re: RFR: JDK-8194762: JTReg failure of "runtime/NMT/PrintNMTStatistics.java"

Okay I'm happy that the test will not accidentally pass if unexpected errors occurs. As the test just does -version then the only errors would be initialization errors that result in a non-zero return code. As the test checks for expected output from NMT and that output is produced as the VM shuts down, then it seems very unlikely for an unrelated error to occur but still see the NMT output and get a zero return code.

So both occurrences of:

output_detail.shouldNotContain("error");

can be removed.

Thanks,
David

On 11/01/2018 12:19 PM, Zhengyu Gu wrote:

> The fix looks fine to me.
>
> On 01/10/2018 05:26 PM, David Holmes wrote:
>> Hi Daniel,
>>
>> I think we will need Zhengyu to chime in and explain what NMT related
>> errors this was intending to catch. The simple search for "error" is
>> obviously too coarse when you may have various logging output enabled.
>> Even this:
>>
>>    64     output_summary.shouldNotContain("error");
>
> I don't think NMT emits any error messages, so this line does not make
> sense.
>
> Thanks,
>
> -Zhengyu
>
>
>>
>> may fail if there is additional logging output.
>>
>> Thanks,
>> David
>>
>> On 11/01/2018 1:28 AM, stewartd.qdt wrote:
>>> Please review this small webrev [1] that removes the search for the
>>> word "error" in stdout for the PrintNMTStatistics during the
>>> NativeMemoryTracking=details sub-test of the JTReg test.
>>>
>>> I have found that on my test machine (AArch64, Linux) when the
>>> detailed list is printed there is a module called "LogOutputList"
>>> that has as one of the listed functions "_dl_catch_error". Since the
>>> HTReg test simply searches for the word "error" in stdout, this test
>>> fails. However, there really isn't a failure to behave correctly and
>>> all the details are printed correctly. It also returns 0 on exit, as
>>> it should. The full log is attached to the bug report.
>>>
>>> I have created this patch in the belief that the test is being
>>> overly restrictive in searching for "error" In stdout and that
>>> printing out "_dl_catch_error" is correct behavior and shouldn't
>>> cause the test to fail.
>>>
>>> Please let me know if this is a bad approach and I'll be happy to
>>> change as required.
>>>
>>> Thanks,
>>> Daniel Stewart
>>>
>>> [1] webrev: http://cr.openjdk.java.net/~dstewart/8194762/webrev.00/
>>> [2] CR: https://bugs.openjdk.java.net/browse/JDK-8194762
>>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8194762: JTReg failure of "runtime/NMT/PrintNMTStatistics.java"

David Holmes
Thanks Daniel. Reviewed and sponsored. Pushed to jdk/hs

David

On 12/01/2018 12:06 AM, stewartd.qdt wrote:

> Thanks, David.
>
> Please see the modified webrev: http://cr.openjdk.java.net/~dstewart/8194762/webrev.01/
>
> Regards,
> Daniel
>
> -----Original Message-----
> From: David Holmes [mailto:[hidden email]]
> Sent: Wednesday, January 10, 2018 9:36 PM
> To: Zhengyu Gu <[hidden email]>; stewartd.qdt <[hidden email]>; [hidden email]
> Subject: Re: RFR: JDK-8194762: JTReg failure of "runtime/NMT/PrintNMTStatistics.java"
>
> Okay I'm happy that the test will not accidentally pass if unexpected errors occurs. As the test just does -version then the only errors would be initialization errors that result in a non-zero return code. As the test checks for expected output from NMT and that output is produced as the VM shuts down, then it seems very unlikely for an unrelated error to occur but still see the NMT output and get a zero return code.
>
> So both occurrences of:
>
> output_detail.shouldNotContain("error");
>
> can be removed.
>
> Thanks,
> David
>
> On 11/01/2018 12:19 PM, Zhengyu Gu wrote:
>> The fix looks fine to me.
>>
>> On 01/10/2018 05:26 PM, David Holmes wrote:
>>> Hi Daniel,
>>>
>>> I think we will need Zhengyu to chime in and explain what NMT related
>>> errors this was intending to catch. The simple search for "error" is
>>> obviously too coarse when you may have various logging output enabled.
>>> Even this:
>>>
>>>     64     output_summary.shouldNotContain("error");
>>
>> I don't think NMT emits any error messages, so this line does not make
>> sense.
>>
>> Thanks,
>>
>> -Zhengyu
>>
>>
>>>
>>> may fail if there is additional logging output.
>>>
>>> Thanks,
>>> David
>>>
>>> On 11/01/2018 1:28 AM, stewartd.qdt wrote:
>>>> Please review this small webrev [1] that removes the search for the
>>>> word "error" in stdout for the PrintNMTStatistics during the
>>>> NativeMemoryTracking=details sub-test of the JTReg test.
>>>>
>>>> I have found that on my test machine (AArch64, Linux) when the
>>>> detailed list is printed there is a module called "LogOutputList"
>>>> that has as one of the listed functions "_dl_catch_error". Since the
>>>> HTReg test simply searches for the word "error" in stdout, this test
>>>> fails. However, there really isn't a failure to behave correctly and
>>>> all the details are printed correctly. It also returns 0 on exit, as
>>>> it should. The full log is attached to the bug report.
>>>>
>>>> I have created this patch in the belief that the test is being
>>>> overly restrictive in searching for "error" In stdout and that
>>>> printing out "_dl_catch_error" is correct behavior and shouldn't
>>>> cause the test to fail.
>>>>
>>>> Please let me know if this is a bad approach and I'll be happy to
>>>> change as required.
>>>>
>>>> Thanks,
>>>> Daniel Stewart
>>>>
>>>> [1] webrev: http://cr.openjdk.java.net/~dstewart/8194762/webrev.00/
>>>> [2] CR: https://bugs.openjdk.java.net/browse/JDK-8194762
>>>>