Looking for a sponsor to review changes made to two unit tests under jdk/test

[David Holmes]

On 7/04/2013 9:37 AM, Mani Sarkar wrote:
> Hi David,
>
> Sorry for not getting back earlier. Here's the changes to
> /jdk8_tl/jdk/test/java/lang/ref/Basic.java that you suggested earlier.

Okay but what about my comment that the latch usage is completely 
unnecessary in the first place ??

David
-----

> -------------------x---------------------
>
> diff -r 16f63a94c231 test/java/lang/ref/Basic.java
> --- a/test/java/lang/ref/Basic.javaFri Apr 05 18:20:12 2013 -0700
> +++ b/test/java/lang/ref/Basic.javaSun Apr 07 00:27:55 2013 +0100
> @@ -29,7 +29,7 @@
>   import java.lang.ref.*;
>   import java.util.Vector;
> -
> +import java.util.concurrent.CountDownLatch;
>   public class Basic {
> @@ -71,10 +71,11 @@
>           });
>       }
> -    static void createNoise() throws InterruptedException {
> +    static void createNoise(final CountDownLatch complete) throws
> InterruptedException {
>           fork(new Runnable() {
>               public void run() {
>                   keep.addElement(new PhantomReference(new Object(), q2));
> +                complete.countDown();
>               }
>           });
>       }
> @@ -101,9 +102,11 @@
>           for (int i = 1;; i++) {
>               Reference r;
> -            createNoise();
> ++           CountDownLatch noiseComplete = new CountDownLatch(1);
> ++           createNoise(noiseComplete);
> +
>               System.err.println("GC " + i);
> -            Thread.sleep(10);
> +            noiseComplete.await();
>               System.gc();
>               System.runFinalization();
> -------------------x---------------------
>
> Its still implemented with CountdownLatch, but as you suggest we
> implement the above via Semaphore it will have to be the next version
> from me - CountdownLatch was suggested by many at the TestFest last
> month but personally I benefit from getting exposed to different
> techniques. I'll back to you with a solution applied using Semaphore.
>
> Cheers,
> mani
>
> On Fri, Apr 5, 2013 at 2:40 AM, David Holmes <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>> wrote:
>
>     On 5/04/2013 11:27 AM, Mani Sarkar wrote:
>
>         Hi David,
>
>         I'll reply in more detail later but to respond to your comment on:
>
>           > I would not add the extra methods around the cdl.await() and
>         cdl.countDown() as they just obscure things
>         In general its meant to do the opposite. We come across a lot of
>         code
>         that does not express intent, and the purpose of encapsulating such
>         blocks (even if its a single line) is to make the flow verbose and
>         readable - I have seen similar code-snippets in the Hotspot (C/C++
>         codebase) making it easy to follow what is going on. One of the
>         things I
>         picked up from TestFest was to make the tests more legible,
>         logical and
>         easy to maintain and scale - it was our intent when we changed
>         this test.
>
>
>     Sorry, createNoiseHasFinishedAddingTo__Keep is certainly verbose but
>     not readable. This would be much more readable:
>
>     Countdownlatch noiseComplete = new ...
>     createNoise(noiseComplete);
>     ...
>     noiseComplete.await();
>
>     and:
>
>     static void createNoise(final CountDownLatch complete) throws
>             InterruptedException {
>         ...
>         complete.countDown();
>     }
>
>     just giving the latch a meaningful name is sufficient to convey intent.
>
>     Note: in this example a Semaphore is probably a better choice than a
>     CountDownLatch.
>
>     Cheers,
>     David
>
>         I'll come back with responses for your other comments.
>
>         Cheers
>         mani
>
>
>
>
> --
> *Twitter:* @theNeomatrix369
> *Blog:* http://neomatrix369.wordpress.com
> *JUG activity:* LJC Advocate (@adoptopenjdk & @adoptajsr programs)
> *Meet-a-Project:* https://github.com/MutabilityDetector
> *Devoxx UK 2013 was a grand success:*
> http://www.devoxx.com/display/UK13/Home
> */Don't chase success, rather aim for "Excellence", and success will
> come chasing after you!/*