Hi Jaroslav,
A couple of questions.
I don't understand why this is a CMS only problem? Why don't the other
collectors have the same issue? I guess it is less likely that the other
collectors start (or complete) a GC without a lot of allocation going
on. But at least G1 should have the same problem.
Also, from the problem description in the CR I would have guessed that
you want the GC to happen between these two statements:
p.setUsageThreshold(1);
MemoryUsage u = p.getUsage();
Now you have added the GC just after these statements. I thought that
was what caused the problem. That you read the usage data at one point,
then a GC happens and you compare the cached usage
data to the new data that p.isUsageThresholdExceeded() will fetch.
Looking at the promoteToOldGen() method I assume that the intent is that
the code should be using the return value. So my guess is that this code:
94 if (p.getName().equals("CMS Old Gen")) {
95 promoteToOldGen(p, u);
96 }
Should be:
94 if (p.getName().equals("CMS Old Gen")) {
95 u = promoteToOldGen(p, u);
96 }
With that, I think it might work. But I still don't understand why this
is only a CMS problem.
One more question about the promoteToOldGen() and forceGC() methods. I
don't really know much about how the different beans work, but are we
sure that the MemoryPoolMXBeans and GarbageCollectorMXBeans use the same
pool names? That is, are you sure that forceGC() actually will do anything?
As for just doing a System.gc() to force a GC I think you can rely on
that System.gc() does a full GC in Hotspot unless someone sets
-XX:+DisableExplicitGC on the command line. Considering that you are
relying on Hotspot specifc names for pools I don't think it is a
limitation to the test to rely on the Hotspot implementatoin of System.gc().
Thanks,
Bengt
On 2013-10-23 10:18, Staffan Larsen wrote:
On 23 okt 2013, at 10:12, Jaroslav Bachorik <jaroslav.bacho...@oracle.com>
wrote:
On 23.10.2013 10:08, Staffan Larsen wrote:
I think you can simplify the logic for forcing a GC to just a simple call to
"System.gc();". AFAIK System.gc() will cause a full collection to happen for
all collectors.
Hm, will it now? I had the impression that it was just hinting the GC system to
perform GC but it might decide to ignore it. I need to be sure that the GC was
performed before continuing - otherwise I might get inconsistent data again.
According to the spec it's just a hint, but I think the implementation happens
to be a force. But better safe than sorry. :)
/Staffan
-JB-
/Staffan
On 23 okt 2013, at 10:02, Jaroslav Bachorik <jaroslav.bacho...@oracle.com>
wrote:
On 22.10.2013 22:04, Mandy Chung wrote:
Hi Jaroslav,
On 10/22/13 6:47 AM, Jaroslav Bachorik wrote:
Please, review the following test fix:
Issue: https://bugs.openjdk.java.net/browse/JDK-8020467
Webrev: http://cr.openjdk.java.net/~jbachorik/8020467/webrev.01
Have you considered to force GC when getUsed() == 0 regardless of which
memory pool it is? This will avoid special casing for CMS old gen in
the test and will handle similar issue in the future for a different
collector implementation. To make the test reliable, the test should
still pass if the memory pool has no object in it (G1 survivor space
case?).
Hi Mandy,
I don't know whether GC will help for other pools - but I can enable it for all
pools - it should not hurt.
The test should pass even with on object in the monitored pool since the pool
should not report an exceeded threshold.
-JB-
Mandy
The test tries to make sure that the "pool usage threshold" trigger
and the reported pool memory usage are not contradicting each other.
The problem is that it is not possible to get the "pool usage
threshold exceeded" flag and the pool memory usage atomicly in regard
to the GC. Specifically, when "CMS Old Gen" pool is examined and the
usage is retrieved before a GC promotes some objects to the old gen
but the usage threshold is checked after the GC has promoted some
instance into the old gen the test will fail.
The patch makes sure that there are some instances promoted in "CMS
Old Gen" before checking the "pool usage threshold" to get
semi-consistent view.
Thanks,
-JB-
