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.ji...@oracle.com
To: mattias.tobias...@oracle.com
Cc: serviceability-dev@openjdk.java.net, daniel.fu...@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.ji...@oracle.com
> To: mattias.tobias...@oracle.com
> Cc: serviceability-dev@openjdk.java.net, daniel.fu...@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.ji...@oracle.com To: daniel.fu...@oracle.com Cc:
> mattias.tobias...@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
>
