Looking for a sponsor to review changes made to two unit tests under jdk/test (original) (raw)
Mani Sarkar sadhak001 at gmail.com
Sat Apr 6 23:37:08 UTC 2013
- Previous message: Looking for a sponsor to review changes made to two unit tests under jdk/test
- Next message: Looking for a sponsor to review changes made to two unit tests under jdk/test
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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. -------------------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 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));
} @@ -101,9 +102,11 @@ for (int i = 1;; i++) { Reference r;complete.countDown(); } });
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>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!
- Previous message: Looking for a sponsor to review changes made to two unit tests under jdk/test
- Next message: Looking for a sponsor to review changes made to two unit tests under jdk/test
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]