RFR: [XS] 8229284: [TESTBUG] jdk/internal/platform/cgroup/TestCgroupMetrics.java fails for - memory:getMemoryUsage

[Bob Vandette]

> On Aug 28, 2019, at 10:18 AM, Baesken, Matthias <matthias.baesken at sap.com> wrote:
> 
>> Should you continue waiting in the loop until BOTH the new usage and max
>> usage exceed their  initial values?
> 
> 
> Hi Bob,  the  current coding  does not do it .  It checks  for  "<"  to fail .
> Please see the current coding :
> 
> 583         long memoryMaxUsage = metrics.getMemoryMaxUsage();
> 584         long memoryUsage = metrics.getMemoryUsage();
> 
> 586         byte[] bb = new byte[64*1024*1024]; // 64M
> 587 
> 588         long newMemoryMaxUsage = metrics.getMemoryMaxUsage();
> 589         long newMemoryUsage = metrics.getMemoryUsage();
> 
> 590 
> 591         if(newMemoryMaxUsage < memoryMaxUsage) {
> 592             fail(SubSystem.MEMORY, "getMemoryMaxUsage", newMemoryMaxUsage,
> 593                     memoryMaxUsage);
> 594         }
> 595 
> 596         if(newMemoryUsage < memoryUsage) {
> 597             fail(SubSystem.MEMORY, "getMemoryUsage", newMemoryUsage, memoryUsage);
> 598         }
> 599     }
> 
> My  change does not intend  to change this logic ,  just does more allocations to make sure the  current check works .

Yes, your change is clearly an improvement.  I just think the original logic is weak and could have been improved at the
same time.  You have a review from Severin so feel free to push what you’ve got.

Bob.


> 
> 
> Best regards, Matthias
> 
> 
> 
>> -----Original Message-----
>> From: Bob Vandette <bob.vandette at oracle.com>
>> Sent: Mittwoch, 28. August 2019 15:24
>> To: Baesken, Matthias <matthias.baesken at sap.com>
>> Cc: Severin Gehwolf <sgehwolf at redhat.com>; hotspot-
>> dev at openjdk.java.net
>> Subject: Re: RFR: [XS] 8229284: [TESTBUG]
>> jdk/internal/platform/cgroup/TestCgroupMetrics.java fails for -
>> memory:getMemoryUsage
>> 
>> Should you continue waiting in the loop until BOTH the new usage and max
>> usage exceed their
>> initial values?
>> 
>> Bob.
>> 
>> 
>>> On Aug 28, 2019, at 8:39 AM, Baesken, Matthias
>> <matthias.baesken at sap.com> wrote:
>>> 
>>> Hi Severin, I changed  the  fail() - calls   and the  newMemoryMaxUsage
>> handling.
>>> New webrev :
>>> 
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8229284.5/
>>> 
>>> 
>>> Best regards, Matthias
>>> 
>>> 
>>>> 
>>>> On Thu, 2019-08-15 at 16:08 +0000, Baesken, Matthias wrote:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8229284
>>>>> 
>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8229284.3/
>>>> 
>>>> Hmm, unrelated to this bug, but still an issue:
>>>> 
>>>> It appears the failure message is bogus when the test fails. It should
>>>> be:
>>>> 
>>>> java.lang.RuntimeException: Test failed for - memory:getMemoryUsage,
>>>> expected [56200986624], got [56019386368]
>>>> 
>>>> I.e. the old/new values need to be swapped here:
>>>> 
>>>> 597             fail(SubSystem.MEMORY, "getMemoryUsage",
>>>> newMemoryUsage, memoryUsage);
>>>> 
>>>> Same for the getMemoryMaxUsage() fail:
>>>> 
>>>> 592             fail(SubSystem.MEMORY, "getMemoryMaxUsage",
>>>> newMemoryMaxUsage,
>>>> 593                     memoryMaxUsage);
>>>> 
>>>> As to the bug. It suggests the memoryUsage code causes problems, not
>>>> memoryMaxUsage.
>>>> 
>>>> 587         // allocate memory in a loop and check more than once for new
>>>> values
>>>> 588         // otherwise we might see seldom the effect of decreasing new
>>>> memory values
>>>> 589         // e.g. because the system could free up memory
>>>> 590         byte[][] bytes = new byte[32][];
>>>> 591         for (int i = 0; i < 32; i++) {
>>>> 592             bytes[i] = new byte[8*1024*1024];
>>>> 593             newMemoryMaxUsage = metrics.getMemoryMaxUsage();
>>>> 594             newMemoryUsage = metrics.getMemoryUsage();
>>>> 595             if (newMemoryMaxUsage > memoryMaxUsage &&
>>>> newMemoryUsage > memoryUsage) {
>>>> 596                 break;
>>>> 597             }
>>>> 
>>>> Do we really need to call metrics.getMemoryMaxUsage() in that loop? I'd
>>>> expect for the following code to work more reliably as the max usage is
>>>> supposed to capture the high water mark. If memoryMaxUsage ever
>>>> decreases, it's a bug in the cgroups accounting system right?
>>>> 
>>>> byte[][] bytes = new byte[32][];
>>>> for (int i = 0; i < 32; i++) {
>>>>   bytes[i] = new byte[8*1024*1024];
>>>>   newMemoryUsage = metrics.getMemoryUsage();
>>>>   if (newMemoryUsage > memoryUsage) {
>>>>       break;
>>>>   }
>>>> }
>>>> newMemoryMaxUsage = metrics.getMemoryMaxUsage();
>>>> 
>>>> // assert things
>>>> 
>>>> Perhaps even assert prior the loop:
>>>> 
>>>> if (memoryMaxUsage < memoryUsage) {
>>>>   throw new RuntimeException("cgroup accounting bug?");
>>>> }
>>>> 
>>>> Thanks,
>>>> Severin
>>> 
>