RFR : JDK-8154166 - java/lang/management/MemoryMXBean/ResetPeakMemoryUsage.java fails with RuntimeException

Harsha Wardhana B harsha.wardhana.b at oracle.com
Mon May 2 06:30:08 UTC 2016


Thanks Jaroslav for the review.

On Monday 02 May 2016 11:49 AM, Jaroslav Bachorik wrote:
> Reviewed.
>
> Thanks!
>
> -JB-
>
> On Mon, May 2, 2016 at 7:23 AM, Harsha Wardhana B 
> <harsha.wardhana.b at oracle.com <mailto:harsha.wardhana.b at oracle.com>> 
> wrote:
>
>     Hi Jaroslav,
>
>     Thanks for pointing out the @required tag. It's a nifty Jtreg feature.
>
>     Below is webrev for updated patch.
>
>     http://cr.openjdk.java.net/~hb/8154166/webrev.02/
>     <http://cr.openjdk.java.net/%7Ehb/8154166/webrev.02/>
>
>     -Harsha
>
>
>     On Friday 29 April 2016 02:15 PM, Jaroslav Bachorik wrote:
>>
>>
>>     On Fri, Apr 29, 2016 at 6:20 AM, Harsha Wardhana B
>>     <harsha.wardhana.b at oracle.com
>>     <mailto:harsha.wardhana.b at oracle.com>> wrote:
>>
>>         Hi Jaroslav,
>>
>>         I am not sure how @required tag works. I searched code base
>>         and it is not used in any file. Also, the documentation on
>>         Jtreg page is sparse.
>>
>>         Could you paste an example as to how to use it?
>>
>>
>>     Please, take a look
>>     at jdk/test/java/lang/management/MemoryMXBean/LowMemoryTest.java
>>     - actually, it is '@requires' tag.
>>
>>
>>         Also, I would still think that repeated gc via weak-reference
>>         is right and defensive approach. So I would like to leave
>>         that in place unless it is causing any side-effects.
>>
>>
>>     No objections here. It does not break anything and makes the test
>>     intentions clearer.
>>
>>     -JB-
>>
>>
>>         Thanks
>>         Harsha
>>
>>
>>         On Tuesday 26 April 2016 04:05 PM, Jaroslav Bachorik wrote:
>>>
>>>
>>>         On Mon, Apr 25, 2016 at 9:27 AM, Harsha Wardhana B
>>>         <harsha.wardhana.b at oracle.com
>>>         <mailto:harsha.wardhana.b at oracle.com>> wrote:
>>>
>>>             Hi,
>>>
>>>             Please review below patch to disable concurrent GC option.
>>>             http://cr.openjdk.java.net/~hb/8154166/webrev.01/
>>>             <http://cr.openjdk.java.net/%7Ehb/8154166/webrev.01/>
>>>
>>>
>>>         I'm sorry to be a PITA, but why it is not possible to use
>>>         the @require tag?
>>>
>>>
>>>
>>>             Jaroslav,
>>>
>>>             According to Javadoc of Runtime.gc(),
>>>
>>>             https://docs.oracle.com/javase/8/docs/api/java/lang/Runtime.html#gc--
>>>
>>>             The call will only make it's best effort to do a GC and
>>>             provides no guarantee that a given object can be
>>>             collected even if GC runs.
>>>             It does not say that Runtime.gc() call will block till
>>>             entire GC cycle is finished and hence we cannot be
>>>             making that assumption.
>>>
>>>
>>>         I know, I had the same discussion a while ago when fixing
>>>         some other tests failing when run with allowed concurrent
>>>         explicit GC and I was pointed to the fact that all the known
>>>         implementation actually do wait until the complete GC cycle
>>>         is over before returning. Otherwise all those tests relying
>>>         on some memory having been reclaimed or some counters having
>>>         been increased would have to be considered random.
>>>
>>>
>>>             Hence it is required that we encapsulate the target
>>>             object in WeakReference and repeatedly call GC till
>>>             weakRef returns null.
>>>             Granted that we will have a small window when weakRef
>>>             returns null and the target object is not removed from
>>>             memory. But I see no way how to fix that problem.
>>>
>>>
>>>         Exactly. The only guarantee for all the GC related metrics
>>>         having been updated before proceeding with the test is being
>>>         able to run the explicit GC in blocking manner. Otherwise
>>>         the tests are not really deterministic and can
>>>         intermittently fail.
>>>
>>>         -JB-
>>>
>>>
>>>             -Harsha
>>>
>>>
>>>             On Sunday 24 April 2016 03:17 PM, Jaroslav Bachorik wrote:
>>>>             The reproducer would be very time sensitive as with the
>>>>             provided 'ExplicitGCInvokesConcurrent' it will run GC
>>>>             concurrently with the invoker. Otherwise, in the
>>>>             current implementation, calling Runtime.gc() would
>>>>             guarantee the GC cycle has finished before that method
>>>>             returns.
>>>>
>>>>             The WeakReference javadoc
>>>>             (https://docs.oracle.com/javase/7/docs/api/java/lang/ref/WeakReference.html)
>>>>             is only stating that the referenced object will be made
>>>>             finalizable at the same time as the reference is
>>>>             cleared. As a consequence a cleared reference might not
>>>>             always mean that the heap usage has been changed
>>>>             (unless a particular GC implementation makes some
>>>>             additional guarantees).
>>>>
>>>>             I know we were stabilizing a bunch of related tests
>>>>             relying on GC doing its work before checking for some
>>>>             post-conditions and the only way to make the tests
>>>>             reliable was to forbid running those tests with
>>>>             '-XX:+ExplicitGCInvokesConcurrent'.
>>>>
>>>>             -JB-
>>>>
>>>>             On Sat, Apr 23, 2016 at 12:15 PM, Harsha Wardhana B
>>>>             <harsha.wardhana.b at oracle.com
>>>>             <mailto:harsha.wardhana.b at oracle.com>> wrote:
>>>>
>>>>                 Hello,
>>>>
>>>>                 The issue was not reproducible with or without,
>>>>
>>>>                 "-XX:+ExplicitGCInvokesConcurrent"
>>>>
>>>>                 Flag. The patch ensures that GC happens before we
>>>>                 start measuring memory. Without the patch, GC might
>>>>                 or might not happen.
>>>>
>>>>                 -Harsha
>>>>
>>>>
>>>>                 On Friday 22 April 2016 07:58 PM, Jaroslav Bachorik
>>>>                 wrote:
>>>>>                 Hi,
>>>>>
>>>>>                 On Fri, Apr 22, 2016 at 9:10 AM, Harsha Wardhana B
>>>>>                 <harsha.wardhana.b at oracle.com
>>>>>                 <mailto:harsha.wardhana.b at oracle.com>> wrote:
>>>>>
>>>>>                     Hi,
>>>>>
>>>>>                     Please review the below simple fix for issue,
>>>>>
>>>>>                     issue :
>>>>>                     https://bugs.openjdk.java.net/browse/JDK-8154166
>>>>>                     webrev :
>>>>>                     http://cr.openjdk.java.net/~hb/8154166/webrev.00/
>>>>>                     <http://cr.openjdk.java.net/%7Ehb/8154166/webrev.00/>
>>>>>
>>>>>
>>>>>                 Shouldn't this test rather declare the conditions
>>>>>                 when it is supposed to work? According to the
>>>>>                 issue this was caused by introducing the
>>>>>                 "-XX:+ExplicitGCInvokesConcurrent" which makes it
>>>>>                 very tricky to make any assumptions about the GC
>>>>>                 process.
>>>>>
>>>>>                 See eg.
>>>>>                 jdk/tests/java/lang/management/MemoryMXBean/LowMemoryTest.java
>>>>>                 for enabling the test only for allowed configurations.
>>>>>
>>>>>                 Cheers,
>>>>>
>>>>>                 -JB-
>>>>>
>>>>>
>>>>>
>>>>>                     -Harsha
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20160502/61f1fe41/attachment-0001.html>


More information about the serviceability-dev mailing list