Looking for a sponsor to review changes made to two unit tests under jdk/test
David Holmes
david.holmes at oracle.com
Mon Apr 8 11:22:01 UTC 2013
On 8/04/2013 7:39 PM, Mani Sarkar wrote:
> 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);
This is wrong. You would want:
final Semaphore noiseComplete = new Semaphore(0);
createNoise(noiseComplete);
noiseComplete.acquire();
but again this is not needed at all.
David
------
> +
> + 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!/*
>>>
>>
>
>
More information about the core-libs-dev
mailing list