jmx-dev RFR 8020467: Inconsistency between usage.getUsed() and isUsageThresholdExceeded() with CMS Old Gen pool

[Bengt Rutisson]

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.bachorik at 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.bachorik at 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-