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/>
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
