[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR(XS) 8031065: LowMemoryTest2.sh fails: OutOfMemoryError: Metaspace
From:       Mattias Tobiasson <mattias.tobiasson () oracle ! com>
Date:       2014-02-28 11:47:11
Message-ID: 69c2acbd-c1a0-47fa-b255-9d110fdeb135 () default
[Download RAW message or body]

Thanks for the review.
Unfortunately I can not remove the thresholdExceeded, because the "break" only leaves \
the loop of memory pools. I know nested loops are not perfect, but I did not want to \
change too much from the original test.

Mattias

----- Original Message -----
From: shanliang.jiang@oracle.com
To: mattias.tobiasson@oracle.com
Cc: serviceability-dev@openjdk.java.net, daniel.fuchs@oracle.com
Sent: Friday, February 28, 2014 11:58:20 AM GMT +01:00 Amsterdam / Berlin / Bern / \
                Rome / Stockholm / Vienna
Subject: Re: RFR(XS) 8031065: LowMemoryTest2.sh fails: OutOfMemoryError: Metaspace

Looks good!

It could be improved to not use the variable thresholdExceeded:

change Line 146 to
    while(true) {
and remove Line 143 and 158

Thanks,
Shanliang


Mattias Tobiasson wrote:
> Hi,
> I have updated the test and now stop allocating when we have reached the threshold.
> Since we now do all allocations first and then just wait for the notification, I \
> have split the loop into two separate loops to make it clearer. 
> To detect if we have reached the threshold I now check \
> MemoryPoolMXBean.getUsageThresholdCount() > 0 instead of checking \
> isUsageThresholdExceeded(). The reason for that is because the notification event \
> is not generated immediately when isUsageThresholdExceeded() = true. The \
> notification is only generated at the next GC. So that is the reason for why the \
> old test kept allocating after it had reached the threshold (to trigger another \
> GC). 
> getUsageThresholdCount() is updated at the same time as the event is generated. So \
> after getUsageThresholdCount() > 0, I can just wait for the notification without \
> more allocations. 
> webrev:
> http://cr.openjdk.java.net/~mtobiass/8031065/webrev.01
> 
> bug:
> https://bugs.openjdk.java.net/browse/JDK-8031065
> 
> Mattias
> 
> ----- Original Message ----- 
> From: shanliang.jiang@oracle.com 
> To: mattias.tobiasson@oracle.com 
> Cc: serviceability-dev@openjdk.java.net, daniel.fuchs@oracle.com 
> Sent: Thursday, February 27, 2014 5:12:51 PM GMT +01:00 Amsterdam / Berlin / Bern / \
>                 Rome / Stockholm / Vienna 
> Subject: Re: RFR(XS) 8031065: LowMemoryTest2.sh fails: OutOfMemoryError: Metaspace 
> 
> 
> Mattias Tobiasson wrote: 
> 
> Hi, thanks for the fast reviews.
> 
> I did think about stop calling loadNext() after the flag has been set. The main \
> reason for not doing that was just because I wanted to change as little as \
> possible. Now the test works as originally intended. I prefer to do like this too \
> :)  
> 
> I do not mind removing the calls to loadNext(), but then we would need some timeout \
> waiting for the callback. Currently the test "times out" with an OutOfMemory when \
> we have allocated the remaining 20% of the space. You do not need to add a timeout, \
> only change Line 151  for(;;) 
> to 
> while(!listenerInvoked) { 
> 
> and remove 160 -- 162 
> 
> in case that an expected notification is not arrived, the testing harness has a \
> timeout to stop the test.  
> This way makes the test more robust, but I am OK with the current fix. 
> 
> Thanks, 
> Shanliang 
> 
> 
> About line 172, you are correct. I will just remove that line. Thanks!
> 
> Mattias
> 
> ----- Original Message -----
> From: shanliang.jiang@oracle.com To: daniel.fuchs@oracle.com Cc: \
> mattias.tobiasson@oracle.com , serviceability-dev@openjdk.java.net Sent: Thursday, \
> February 27, 2014 12:59:49 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / \
>                 Stockholm / Vienna
> Subject: Re: RFR(XS) 8031065: LowMemoryTest2.sh fails: OutOfMemoryError: Metaspace
> 
> Daniel Fuchs wrote: 
> 
> On 2/27/14 11:43 AM, Mattias Tobiasson wrote: 
> 
> Hi,
> Could you please review this test fix.
> 
> The test verifies that MemoryPoolMXBean sends a notification when 
> used memory has reached the threshold.
> The flag thresholdExceeded marks if we have reached the memory 
> threshold. When the flag is set, the test slows down to give time for 
> the notification to be received.
> The problem is that thresholdExceeded is overwritten every time in 
> the loop. Instead it should be set if any pool has reached the 
> threshold. This means that the test continues to allocate memory at 
> full speed, and we may get an OutOfMemory before we get the 
> notification. Hi Mattias,
> 
> I wonder whether you should also stop calling loadNext() once
> thresholdExceeded is true? Yes I am thinking this too.
> 
> Line 172 is unnecessary, after thresholdExceeded becomes true, Line 170 
> will always be skipped.
> 
> Shanliang 
> 
> best regards,
> 
> -- daniel 
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8031065 webrev: \
> http://cr.openjdk.java.net/~ykantser/8031065/webrev.00/ Mattias  


[prev in list] [next in list] [prev in thread] [next in thread]