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.wardhan...@oracle.com <mailto: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/
    <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.wardhan...@oracle.com
    <mailto: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
        <mailto:harsha.wardhan...@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.wardhan...@oracle.com
            <mailto: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
                <mailto:harsha.wardhan...@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











Reply via email to