Reviewed. Thanks!
-JB- On Mon, May 2, 2016 at 7:23 AM, Harsha Wardhana B < harsha.wardhan...@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/ > > -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.wardhan...@oracle.com>harsha.wardhan...@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.wardhan...@oracle.com> wrote: >> >>> Hi, >>> >>> Please review below patch to disable concurrent GC option. >>> http://cr.openjdk.java.net/~hb/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> >>> 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.wardhan...@oracle.com>harsha.wardhan...@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.wardhan...@oracle.com>harsha.wardhan...@oracle.com> wrote: >>>> >>>>> Hi, >>>>> >>>>> Please review the below simple fix for issue, >>>>> >>>>> issue : <https://bugs.openjdk.java.net/browse/JDK-8154166> >>>>> https://bugs.openjdk.java.net/browse/JDK-8154166 >>>>> webrev : <http://cr.openjdk.java.net/%7Ehb/8154166/webrev.00/> >>>>> http://cr.openjdk.java.net/~hb/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 >>>>> >>>>> >>>> >>>> >>> >>> >> >> > >
