Re: RFR: 8258584: java/util/HexFormat/HexFormatTest.java fails on x86_32 [v2]
On Mon, 21 Dec 2020 15:34:16 GMT, Roger Riggs wrote:
>> Jie Fu has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Ignore OOME for testOOME
>> - Revert the change
>
> Completely ignoring the exception will leave no trace that the test was
> skipped or why.
> Please retain the printing of the memory limits and instead of rethrowing the
> oome add:
> new SkipException("Insufficient Memory to test OOME");```
> (It will need an import of org.testng.SkipException).
> Throwing SkipException will flag the test as being skipped in the Jtreg
> summary.
> Completely ignoring the exception will leave no trace that the test was
> skipped or why.
> Please retain the printing of the memory limits and instead of rethrowing the
> oome add:
> `throw new SkipException("Insufficient Memory to test OOME");`
> (It will need an import of org.testng.SkipException).
> Throwing SkipException will flag the test as being skipped in the Jtreg
> summary.
Updated.
Thanks for your help.
-
PR: https://git.openjdk.java.net/jdk/pull/1817
Re: RFR: 8258584: java/util/HexFormat/HexFormatTest.java fails on x86_32 [v2]
On Sat, 19 Dec 2020 09:45:09 GMT, Jie Fu wrote:
>> Hi all,
>>
>> java/util/HexFormat/HexFormatTest.java fails on x86_32 due to '-Xmx4G'.
>> The reason is that -Xmx4G is invalid maximum heap size for 32-bit platforms.
>> The current implementation only supports maximum 3800M on 32-bit systems [1].
>>
>> I've tried to reduce the -Xmx size, but it still fails even with -Xmx2G.
>> So this test seems to be brittle on 32-bit platforms since 2G is already
>> larger than 3800M/2=1900M.
>> The fix just skips the test for 32-bit systems.
>>
>> Thanks.
>> Best regards,
>> Jie
>>
>> [1]
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/os/posix/os_posix.cpp#L567
>
> Jie Fu has updated the pull request incrementally with two additional commits
> since the last revision:
>
> - Ignore OOME for testOOME
> - Revert the change
Completely ignoring the exception will leave no trace that the test was skipped
or why.
Please retain the printing of the memory limits and instead of rethrowing the
oome add:
new SkipException("Insufficient Memory to test OOME");```
(It will need an import of org.testng.SkipException).
Throwing SkipException will flag the test as being skipped in the Jtreg summary.
-
Changes requested by rriggs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/1817
Re: RFR: 8258584: java/util/HexFormat/HexFormatTest.java fails on x86_32 [v2]
On Thu, 17 Dec 2020 16:14:56 GMT, Roger Riggs wrote: > Disabling all of the tests on 32 bit is not a good idea. > > Instead the HexFormatTest.testOOME test should be skipped or the OOME should > be ignored. > Checking Runtime.getRuntime().maxMemory() should provide enough info to skip > it. Thanks @RogerRiggs for your review and comments. Let's ignore the OOME for testOOME. What do you think of the updated change? Thanks. - PR: https://git.openjdk.java.net/jdk/pull/1817
Re: RFR: 8258584: java/util/HexFormat/HexFormatTest.java fails on x86_32 [v2]
> Hi all, > > java/util/HexFormat/HexFormatTest.java fails on x86_32 due to '-Xmx4G'. > The reason is that -Xmx4G is invalid maximum heap size for 32-bit platforms. > The current implementation only supports maximum 3800M on 32-bit systems [1]. > > I've tried to reduce the -Xmx size, but it still fails even with -Xmx2G. > So this test seems to be brittle on 32-bit platforms since 2G is already > larger than 3800M/2=1900M. > The fix just skips the test for 32-bit systems. > > Thanks. > Best regards, > Jie > > [1] > https://github.com/openjdk/jdk/blob/master/src/hotspot/os/posix/os_posix.cpp#L567 Jie Fu has updated the pull request incrementally with two additional commits since the last revision: - Ignore OOME for testOOME - Revert the change - Changes: - all: https://git.openjdk.java.net/jdk/pull/1817/files - new: https://git.openjdk.java.net/jdk/pull/1817/files/6b32101d..38d4d01a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1817=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1817=00-01 Stats: 8 lines in 1 file changed: 0 ins; 6 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1817.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1817/head:pull/1817 PR: https://git.openjdk.java.net/jdk/pull/1817
