Hi Mandy,
made multi-line @summary as last tag.
yes it was simpler before and we can use '@requires only 64 bit', but we
won't be able to verify it on Linux 32 bit in this case.
As it is possible to run -Xmx3000M on Linux 32 bit machines most of the
time but can't guarantee it all the time.
Hence used ProcessTools.executeTestJava() to make decision about using
-Xmx3000M.
I think it would be better if we can make Test run on Linux 32 bit
machine also.
webrev :
http://cr.openjdk.java.net/~uvangapally/webrev/2017/8178508/webrev.02/
Thanks,
Ujwal.
On 6/13/2017 7:28 AM, Mandy Chung wrote:
On Jun 8, 2017, at 4:13 AM, Ujwal Vangapally
<[email protected] <mailto:[email protected]>> wrote:
Thanks for the Review Mandy, Igor, Harsha.
below is the link for new webrev incorporating review comments.
webrev:
http://cr.openjdk.java.net/~uvangapally/webrev/2017/8178508/webrev.01/
<http://cr.openjdk.java.net/%7Euvangapally/webrev/2017/8178508/webrev.00/>
As you originally had, using @run main/othervm -Xmx3000M
LargeHeapThresholdTest is much simpler. I think it’s better to say
@requires to run only only 64-bit platforms.
Can you move the multi-line @summary to the last tag.
Mandy
changes in new webrev other than those mentioned in review comments:
using -Xmx3000M option on unsupported machines(like window 32bit
machine containing just 2GB user space) creates unnecessary failures
in previous implementation, so used ProcessTools.executeTestJava() to
do a sample run of "java -Xmx3000M -version" and took decision
depending on it's exit value whether to use -Xmx3000M or not in
actual execution .
removed '@requires os.maxmemory >3G' as we really don't require
more than 3GB physical memory to verify this test.
Suggestions:
will it makes more sense to add
@requires (os.family != "windows") | (os.simpleArch != "i586")
as windows 32 bit just provides 2GB for user space.
no problem with current test as it just prints
"Ability to use big heap thresholds has NOT been verified"
on win 32bit.
Thanks,
Ujwal.
On 6/5/2017 10:29 PM, Mandy Chung wrote:
On Jun 5, 2017, at 9:48 AM, Mandy Chung <[email protected]
<mailto:[email protected]>> wrote:
On 5/31/2017 10:32 AM, Ujwal Vangapally wrote:
webrev :
http://cr.openjdk.java.net/~uvangapally/webrev/2017/8178508/webrev.00/
<http://cr.openjdk.java.net/%7Euvangapally/webrev/2017/8178508/webrev.00/>
The test should be in test/java/lang/management/MemoryPoolMXBean
since it’s a test for that API. I also suggest to rename the test
to LargeHeapThresholdTest (or something like that).
41 * @run main/othervm -Xmx3000M -ea MX2G
59 assert i.getUsageThreshold() == TWO_G : "Usage threshold
for"
The test should elimiate its dependency on -ea; otherwise the test
may not fail when it runs standalone without -ea. You can eplace
the assert with an if-statement to check the condition and throw a
RuntimeException instead.
Mandy
