Looking for a sponsor to review changes made to two unit tests under jdk/test
David Holmes
david.holmes at oracle.com
Fri Apr 5 01:07:49 UTC 2013
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 (sadhak001 at gmail.com <mailto:sadhak001 at gmail.com>) and
> Edward Yue Shung Wong (edward.ys.wong at gmail.com
> <mailto:edward.ys.wong at 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));
> + createNoiseHasFinishedAddingToKeep(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);
> + waitUntilCreateNoiseHasFinished(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
> createNoiseHasFinishedAddingToKeep(CountDownLatch cdl) {
> + cdl.countDown();
> + }
> +
> + private static void waitUntilCreateNoiseHasFinished(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.wong at gmail.com
> <mailto:edward.ys.wong at 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.holmes at oracle.com
> <mailto:david.holmes at 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
> <sadhak001 at gmail.com <mailto:sadhak001 at 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>
> *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