Hi Jesper,
On 2014-09-09 15:08, Jesper Wilhelmsson wrote:
Hi Stefan,
g1Flag is only used in one place. In my opinion it doesn't add any
value having it as a variable rather than just putting the flag
directly where it is used (keeping the comment of course):
RunUtil.runTestClearGcOpts(main, nmFlag, lpFlag, "-XX:+UseSerialGC");
RunUtil.runTestClearGcOpts(main, nmFlag, lpFlag, "-XX:+UseParallelGC");
RunUtil.runTestClearGcOpts(main, nmFlag, lpFlag,
"-XX:+UseConcMarkSweepGC");
// Prevent G1 from selecting a large heap region size,
// since that would change the young gen size.
RunUtil.runTestClearGcOpts(main, nmFlag, lpFlag, "-XX:+UseG1GC",
"-XX:G1HeapRegionSize=1m");
I think my version makes the documentation of that flag more explicit
and makes it easier to read the list of run targets. I'll prefer to keep
it as it is.
Besides that, ship it!
thanks,
StefanK
/Jesper
Stefan Karlsson skrev 9/9/14 14:29:
Hi Jaroslav,
Thanks a lot for looking at the test change.
On 2014-09-09 14:04, Jaroslav Bachorik wrote:
Hi Stefan,
* test/java/lang/management/MemoryMXBean/LowMemoryTest.java
L71 - `g1Flag` is not used anywhere; should it be removed?
It should be used. I've updated the patch:
http://cr.openjdk.java.net/~stefank/8057174/webrev.02
and have restarted the test runs.
thanks,
StefanK
Other than that the change looks fine.
-JB-
On 09/09/2014 01:45 PM, Stefan Karlsson wrote:
(Adding GC)
Hi,
Could I get a couple of reviews for this test fix?
StefanK
On 2014-09-05 14:01, Stefan Karlsson wrote:
Hi all,
Here's an updated version of the test:
http://cr.openjdk.java.net/~stefank/8057174/webrev.01.delta/
http://cr.openjdk.java.net/~stefank/8057174/webrev.01/
1) The max == -1 check would unnecessarily fail the test (if it ever
would happen).
2) Some flags/configurations could increase the young gen size
because
of alignment restrictions in the heap sizing policies. I've updated
the test to make sure that we run
a) with small G1 heap region sizes
b) without large pages
I had to remove the RunUtil.runTestKeepGcOpts targets to minimize the
risk that someone will add arbitrary GC command line options to
the test.
thanks,
StefanK
On 2014-09-04 16:34, Stefan Karlsson wrote:
Hi all,
Please review this patch to make these tests a bit more stable. I've
changed the code to always allocate objects that are larger than the
young gen size.
http://cr.openjdk.java.net/~stefank/8057174/webrev.00/
I've tested this by running the tests through jprt.
thanks,
StefanK
