Looking for a sponsor to review changes made to two unit tests under jdk/test (original) (raw)

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.javaimplemented 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 @@ }); }

InterruptedException { fork(new Runnable() { public void run() { keep.addElement(new PhantomReference(new Object(), q2));

----------------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 /jdk8tl/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.%2A%2Acomhttp://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