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

Mani Sarkar sadhak001 at gmail.com
Mon Apr 8 09:39:30 UTC 2013


Hi David,

Here's the version of
*jdk8_tl/jdk/**test/java/lang/ref/Basic.java*implemented using a
Semaphore:

----------------x-------------
diff -r 16f63a94c231 test/java/lang/ref/Basic.java
--- a/test/java/lang/ref/Basic.java Fri Apr 05 18:20:12 2013 -0700
+++ b/test/java/lang/ref/Basic.java Sun Apr 07 11:06:17 2013 +0100
@@ -29,7 +29,7 @@

 import java.lang.ref.*;
 import java.util.Vector;
-
+import java.util.concurrent.Semaphore;

 public class Basic {

@@ -71,10 +71,11 @@
         });
     }

-    static void createNoise() throws InterruptedException {
+    static void createNoise(final Semaphore complete) throws
InterruptedException {
         fork(new Runnable() {
             public void run() {
                 keep.addElement(new PhantomReference(new Object(), q2));
+                complete.release();
             }
         });
     }
@@ -101,9 +102,12 @@
         for (int i = 1;; i++) {
             Reference r;

-            createNoise();
-            System.err.println("GC " + i);
-            Thread.sleep(10);
+            final Semaphore noiseComplete = new Semaphore(1);
+            noiseComplete.acquire();
+
+            createNoise(noiseComplete);
+
+            System.err.println("GC " + i);
             System.gc();
             System.runFinalization();
 ----------------x-------------

After running the test the first time I added temporary logging statements
between the blocks of code that dealt with the Semaphore and got the below
results:

*----------System.out:(12/444)----------*
*> Semaphore created.*
*> Semaphore acquired.*
*>> Inside createnoise(): before adding an element.*
*>> Inside createnoise(): after adding an element.*
*>> Inside createnoise(): Semaphore released.*
*> After createnoise() is called.*
*> Semaphore created.*
*> Semaphore acquired.*
*>> Inside createnoise(): before adding an element.*
*>> Inside createnoise(): after adding an element.*
*>> Inside createnoise(): Semaphore released.*
*> After createnoise() is called.*

What is the principle difference between CountdownLatch and Semaphore and
why is a later more applicable in this context than the former? The
mehanism by which they handle the two threads appears the same to me, could
you please explain to satisfy my concurrency curiosity.

Regards,
mani

On Mon, Apr 8, 2013 at 3:00 AM, David Holmes <david.holmes at oracle.com>wrote:

> 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 <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<http://neomatrix369.wordpress.com>
>> *JUG activity:* LJC Advocate (@adoptopenjdk & @adoptajsr programs)
>> *Meet-a-Project:* https://github.com/**MutabilityDetector<https://github.com/MutabilityDetector>
>> *Devoxx UK 2013 was a grand success:*
>> http://www.devoxx.com/display/**UK13/Home<http://www.devoxx.com/display/UK13/Home>
>> */Don't chase success, rather aim for "Excellence", and success will
>> come chasing after you!/*
>>
>


-- 
*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!*



More information about the core-libs-dev mailing list