On Fri, Apr 29, 2016 at 6:20 AM, Harsha Wardhana B < 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>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 >>>> >>>> >>> >>> >> >> > >