Hi Bengt,

On 23.10.2013 14:40, Bengt Rutisson wrote:

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.

I don't really know. If there are other pools that can have the "used" value 0 before a GC happens then yes, they are susceptible to 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();

This is all but a heuristic here. The problem lies in the fact that it is not possible to retrieve the pool usage and the "threshold exceeded" flag consistently in one atomic operation. I might get usable data from the first call and then I don't need to force GC.


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         }

Indeed. It was meant to re-fetch the usage after GC.


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?

They use the pool names as reported by the GC infrastracture so they should be the same.


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().

Good to know. I guess I could simplify the change and just call System.gc(), after all.

Thanks,

-JB-


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-


Reply via email to