Please use the below link for new webrev, link in my previous reply is
redirecting to old webrev
webrev :
http://cr.openjdk.java.net/~uvangapally/webrev/2017/8178508/webrev.01/
Thanks,
Ujwal.
On 6/8/2017 4:43 PM, Ujwal Vangapally 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/>
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
