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, createNoiseHasFinishedAddingToKeep 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
On Fri, Apr 5, 2013 at 2:07 AM, David Holmes <david.hol...@oracle.com
<mailto:david.hol...@oracle.com>> wrote:
Hi Mani,
Patches came through ok.
I would not add the extra methods around the cdl.await() and
cdl.countDown() as they just obscure things in my opinion. But that
aside the latch is not needed. The fork() method starts a thread and
joins it. So when createNoise() returns we already know for certain
that the "noise has been created". What the sleep is doing is giving
the GC a chance to run.
David
On 5/04/2013 10:55 AM, Mani Sarkar wrote:
Thanks David,
Here are the patches, let me know if they have come in fine:
1) test/java/lang/ref/Basic.____java.patch - changed to not use
Thread.sleep() any more rather use the
java.util.concurrent.____CountdownLatch
functionality
Authors: Mani (sadhak...@gmail.com <mailto:sadhak...@gmail.com>
<mailto:sadhak...@gmail.com <mailto:sadhak...@gmail.com>>) and
Edward Yue Shung Wong (edward.ys.w...@gmail.com
<mailto:edward.ys.w...@gmail.com>
<mailto:edward.ys.wong@gmail.__com
<mailto:edward.ys.w...@gmail.com>>)
------------x---------------
diff -r 38e1821c4472 test/java/lang/ref/Basic.java
--- a/test/java/lang/ref/Basic.__javaWed Mar 06 18:35:51 2013 +0100
+++ b/test/java/lang/ref/Basic.__javaSat Mar 23 14:51:25 2013 +0000
@@ -29,7 +29,7 @@
import java.lang.ref.*;
import java.util.Vector;
-
+import java.util.concurrent.__CountDownLatch;
public class Basic {
@@ -64,22 +64,22 @@
fork(new Runnable() {
public void run() {
System.err.println("__References: W " + rw.get()
- + ", W2 " + rw2.get()
- + ", P " + rp.get()
- + ", P2 " + rp2.get());
+ + ", W2 " + rw2.get()
+ + ", P " + rp.get()
+ + ", P2 " + rp2.get());
}
});
}
- static void createNoise() throws InterruptedException {
+ static void createNoise(final CountDownLatch cdl) throws
InterruptedException {
fork(new Runnable() {
public void run() {
keep.addElement(new PhantomReference(new
Object(), q2));
+ createNoiseHasFinishedAddingTo__Keep(cdl);
}
});
}
-
public static void main(String[] args) throws Exception {
fork(new Runnable() {
@@ -97,13 +97,16 @@
int ndq = 0;
boolean prevFinalized = false;
- outer:
+ outer:
for (int i = 1;; i++) {
Reference r;
- createNoise();
+ CountDownLatch inQueueWaitLatch = new
CountDownLatch(1);
+ createNoise(inQueueWaitLatch);
+
System.err.println("GC " + i);
- Thread.sleep(10);
+ waitUntilCreateNoiseHasFinishe__d(inQueueWaitLatch);
+
System.gc();
System.runFinalization();
@@ -137,7 +140,7 @@
if (ndq != 3) {
throw new Exception("Expected to dequeue 3
reference objects,"
- + " but only got " + ndq);
+ + " but only got " + ndq);
}
if (! Basic.finalized) {
@@ -146,4 +149,13 @@
}
+
+ private static void
createNoiseHasFinishedAddingTo__Keep(CountDownLatch cdl) {
+ cdl.countDown();
+ }
+
+ private static void
waitUntilCreateNoiseHasFinishe__d(CountDownLatch
cdl) throws InterruptedException {
+ cdl.await();
+ }
+
}
------------x----------------
2) test/java/lang/Runtime/exec/____LotsOfOutput.java.patch -
refactor-ing
and tidy-ing of existing code (removing string literals and
replacing
with constants, etc...)
Author: Edward Yue Shung Wong (edward.ys.w...@gmail.com
<mailto:edward.ys.w...@gmail.com>
<mailto:edward.ys.wong@gmail.__com
<mailto:edward.ys.w...@gmail.com>>)
------------x----------------
diff -r 38e1821c4472 test/java/lang/Runtime/exec/__LotsOfOutput.java
--- a/test/java/lang/Runtime/exec/__LotsOfOutput.javaWed Mar 06
18:35:51
2013 +0100
+++ b/test/java/lang/Runtime/exec/__LotsOfOutput.javaSat Mar 23
15:48:46
2013 +0000
@@ -33,17 +33,24 @@
public class LotsOfOutput {
static final String CAT = "/usr/bin/cat";
- public static void main(String[] args) throws Exception{
- if (File.separatorChar == '\\' || // Windows
- !new File(CAT).exists()) // no cat
+ static final int MEMORY_GROWTH_LIMIT = 1000000;
+
+ public static void main(String[] args) throws Exception {
+ if (isWindowsOrCatNotAvailable()) {
return;
+ }
+
Process p = Runtime.getRuntime().exec(CAT + "
/dev/zero");
long initMemory = Runtime.getRuntime().__totalMemory();
- for (int i=1; i< 10; i++) {
+ for (int i = 1; i < 10; i++) {
Thread.sleep(100);
- if (Runtime.getRuntime().__totalMemory() >
initMemory + 1000000)
- throw new Exception("Process consumes memory.");
+ if (Runtime.getRuntime().__totalMemory() > initMemory +
MEMORY_GROWTH_LIMIT)
+ throw new Exception("Runtime memory has grown more
than: " + MEMORY_GROWTH_LIMIT);
}
}
+
+ private static boolean isWindowsOrCatNotAvailable() {
+ return File.separatorChar == '\\' || !new
File(CAT).exists();
+ }
}
------------x----------------
Any queries please let me know.
Thanks.
Regards,
mani
On Fri, Apr 5, 2013 at 1:38 AM, David Holmes
<david.hol...@oracle.com <mailto:david.hol...@oracle.com>
<mailto:david.holmes@oracle.__com
<mailto:david.hol...@oracle.com>>> wrote:
Hi Mani,
Attachments get stripped so you will need to inline the
patches in
your email to the list.
Thanks,
David
On 5/04/2013 9:07 AM, Mani Sarkar wrote:
Hi all,
I'd like to rectify that I;m a contributor (and not a
committer as
mentioned earlier), so I don't have access to the
webrev system
but would
love to have these patches hosted on them when a
sponsor becomes
available.
Cheers,
mani
On Thu, Apr 4, 2013 at 11:57 PM, Mani Sarkar
<sadhak...@gmail.com <mailto:sadhak...@gmail.com>
<mailto:sadhak...@gmail.com <mailto:sadhak...@gmail.com>>> wrote:
Hi all,
During #TestFest in London last month we hacked away on
jtreg and tests in
the OpenJDK. Myself and another participant managed to
change two unit
tests. I haven't looked for a sponsor in the past
so I'm
fairly new to the
process, hence my email on the list is to request for
someone to review,
and process these patches further. I'm a committer
(signed OCA).
Here's details of the changes made to the tests
under the
.*./jdk8_tl/jdk*folders:
1) *test/java/lang/ref/Basic.____java.patch* -
changed to not use
Thread.sleep() any more rather use the
java.util.concurrent.____CountdownLatch
functionality
2)
*test/java/lang/Runtime/exec/____LotsOfOutput.java.patch* -
refactor-ing
and tidy-ing of existing code (removing string
literals and
replacing with
constants, etc...)
Please let me know what you would like me to do
next with
these patches.
Regards,
mani
--
*Twitter:* @theNeomatrix369
*Blog:* http://neomatrix369.wordpress.____com
<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>
<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>
<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
<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!/*
