Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently

2014-01-07 Thread David Holmes

On 8/01/2014 4:19 PM, David Holmes wrote:

On 8/01/2014 7:33 AM, srikalyan chandrashekar wrote:

Hi David, TraceExceptions with fastdebug build produced some nice trace
 . The
native method wait(long) is where the OOME if being thrown, the deepest
call is in

src/share/vm/gc_interface/collectedHeap.inline.hpp, line 157


Yes but it is the caller that is of interest:

Exception  (0xd6a01840)
thrown
[/HUDSON/workspace/8-2-build-linux-amd64/jdk8/1317/hotspot/src/share/vm/runtime/objectMonitor.cpp,
line 1649]
for thread 0x7f78c40d2800
Exception  (0xd6a01840)
  thrown in interpreter method <{method} {0x7f78b4800ae0} 'wait'
'(J)V' in 'java/lang/Object'>
  at bci 0 for thread 0x7f78c40d2800

The ReferenceHandler thread gets the OOME trying to allocate the
InterruptedException.


However we already have a catch block around the wait() so how is this 
OOME getting through? A bug in exception handling in the interpreter ??


David


David
-


--- Excerpt Begins -

147  if (!gc_overhead_limit_was_exceeded) {
148// -XX:+HeapDumpOnOutOfMemoryError and -XX:OnOutOfMemoryError
support
149report_java_out_of_memory("Java heap space");
150
151if (JvmtiExport::should_post_resource_exhausted()) {
152  JvmtiExport::post_resource_exhausted(
153JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR |
JVMTI_RESOURCE_EXHAUSTED_JAVA_HEAP,
154"Java heap space");
155}
156
157THROW_OOP_0(Universe::out_of_memory_error_java_heap());
158  } else {

--- Excerpt Ends -


Would be helpful if David/some one else in the team could explain the
latent aspects/probable cause.

---
Thanks
kalyan

On 01/06/2014 04:40 PM, David Holmes wrote:

Back from vacation ...

On 20/12/2013 4:49 PM, David Holmes wrote:

On 20/12/2013 12:57 PM, srikalyan chandrashekar wrote:

Hi David Thanks for your comments, the unguarded part(clean and
enqueue)
in the Reference Handler thread does not seem to create any new
objects,
so it is the application(the test in this case) which is adding
objects
to heap and causing the Reference Handler to die with OOME.


The ReferenceHandler thread can only get OOME if it allocates (directly
or indirectly) - so there has to be something in the unguarded part
that
causes this. Again it may be an implicit action in the VM - similar to
the class load issue for InterruptedException.


Run a debug VM with -XX:+TraceExceptions to see where the OOME is
triggered.

David
-


David

I am still

unsure about the side effects of the code change and agree with your
thoughts(on memory exhaustion test's reliability).

PS: hotspot dev alias removed from CC.

--
Thanks
kalyan

On 12/19/13 5:08 PM, David Holmes wrote:

Hi Kalyan,

This is not a hotspot issue so I'm moving this to core-libs, please
drop hotspot from any replies.

On 20/12/2013 6:26 AM, srikalyan wrote:

Hi all,  I have been working on the bug JDK-8022321
 , this is a
sporadic
failure and the webrev is available here
http://cr.openjdk.java.net/~srikchan/Regression/JDK-8022321_OOMEInReferenceHandler-webrev/






I'm really not sure what to make of this. We have a test that
triggers
an out-of-memory condition but the OOME can actually turn up in the
ReferenceHandler thread causing it to terminate and the test to fail.
We previously accounted for the non-obvious occurrences of OOME
due to
the Object.wait and the possible need to load the
InterruptedException
class - but still the OOME can appear where we don't want it. So
finally you have just placed the whole for(;;) loop in a
try/catch(OOME) that ignores the OOME. I'm certain that makes the
test
happy, but I'm not sure it is really what we want for the
ReferenceHandler thread. If the OOME occurs while cleaning, or
enqueuing then we will fail to clean and/or enqueue but there
would be
no indication that has occurred and I think that is a bigger problem
than this test failing.

There may be no way to make this test 100% reliable. In fact I'd
suggest that no memory exhaustion test can be 100% reliable.

David


*
**"Root Cause:Still not known"*
2 places where there is a possibility for OOME
1) Cleaner.clean()
2) ReferenceQueue.enqueue()

1)  The cleanup code in turn has 2 places where there is potential
for
throwing OOME,
 a) thunk Thread which is run from clean() method. This
Runnable is
passed to Cleaner and appears in the following classes
 java/nio/DirectByteBuffer.java
 sun/misc/Perf.java
 sun/nio/fs/NativeBuffer.java
 sun/nio/ch/IOVecWrapper.java
 sun/misc/Cleaner/ExitOnThrow.java
However none of the above overridden implementations ever create an
object in the clean() code.
 b) new PrivilegedAction created in try catch Exception block of
clean() method but for this object to be created and to be held
responsible for OOME an Exception(other tha

Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently

2014-01-07 Thread David Holmes

On 8/01/2014 7:33 AM, srikalyan chandrashekar wrote:

Hi David, TraceExceptions with fastdebug build produced some nice trace
 . The
native method wait(long) is where the OOME if being thrown, the deepest
call is in

src/share/vm/gc_interface/collectedHeap.inline.hpp, line 157


Yes but it is the caller that is of interest:

Exception  (0xd6a01840)
thrown 
[/HUDSON/workspace/8-2-build-linux-amd64/jdk8/1317/hotspot/src/share/vm/runtime/objectMonitor.cpp, 
line 1649]

for thread 0x7f78c40d2800
Exception  (0xd6a01840)
 thrown in interpreter method <{method} {0x7f78b4800ae0} 'wait' 
'(J)V' in 'java/lang/Object'>

 at bci 0 for thread 0x7f78c40d2800

The ReferenceHandler thread gets the OOME trying to allocate the 
InterruptedException.


David
-


--- Excerpt Begins -

147  if (!gc_overhead_limit_was_exceeded) {
148// -XX:+HeapDumpOnOutOfMemoryError and -XX:OnOutOfMemoryError
support
149report_java_out_of_memory("Java heap space");
150
151if (JvmtiExport::should_post_resource_exhausted()) {
152  JvmtiExport::post_resource_exhausted(
153JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR |
JVMTI_RESOURCE_EXHAUSTED_JAVA_HEAP,
154"Java heap space");
155}
156
157THROW_OOP_0(Universe::out_of_memory_error_java_heap());
158  } else {

--- Excerpt Ends -


Would be helpful if David/some one else in the team could explain the
latent aspects/probable cause.

---
Thanks
kalyan

On 01/06/2014 04:40 PM, David Holmes wrote:

Back from vacation ...

On 20/12/2013 4:49 PM, David Holmes wrote:

On 20/12/2013 12:57 PM, srikalyan chandrashekar wrote:

Hi David Thanks for your comments, the unguarded part(clean and
enqueue)
in the Reference Handler thread does not seem to create any new
objects,
so it is the application(the test in this case) which is adding objects
to heap and causing the Reference Handler to die with OOME.


The ReferenceHandler thread can only get OOME if it allocates (directly
or indirectly) - so there has to be something in the unguarded part that
causes this. Again it may be an implicit action in the VM - similar to
the class load issue for InterruptedException.


Run a debug VM with -XX:+TraceExceptions to see where the OOME is
triggered.

David
-


David

I am still

unsure about the side effects of the code change and agree with your
thoughts(on memory exhaustion test's reliability).

PS: hotspot dev alias removed from CC.

--
Thanks
kalyan

On 12/19/13 5:08 PM, David Holmes wrote:

Hi Kalyan,

This is not a hotspot issue so I'm moving this to core-libs, please
drop hotspot from any replies.

On 20/12/2013 6:26 AM, srikalyan wrote:

Hi all,  I have been working on the bug JDK-8022321
 , this is a
sporadic
failure and the webrev is available here
http://cr.openjdk.java.net/~srikchan/Regression/JDK-8022321_OOMEInReferenceHandler-webrev/





I'm really not sure what to make of this. We have a test that triggers
an out-of-memory condition but the OOME can actually turn up in the
ReferenceHandler thread causing it to terminate and the test to fail.
We previously accounted for the non-obvious occurrences of OOME due to
the Object.wait and the possible need to load the InterruptedException
class - but still the OOME can appear where we don't want it. So
finally you have just placed the whole for(;;) loop in a
try/catch(OOME) that ignores the OOME. I'm certain that makes the test
happy, but I'm not sure it is really what we want for the
ReferenceHandler thread. If the OOME occurs while cleaning, or
enqueuing then we will fail to clean and/or enqueue but there would be
no indication that has occurred and I think that is a bigger problem
than this test failing.

There may be no way to make this test 100% reliable. In fact I'd
suggest that no memory exhaustion test can be 100% reliable.

David


*
**"Root Cause:Still not known"*
2 places where there is a possibility for OOME
1) Cleaner.clean()
2) ReferenceQueue.enqueue()

1)  The cleanup code in turn has 2 places where there is potential
for
throwing OOME,
 a) thunk Thread which is run from clean() method. This
Runnable is
passed to Cleaner and appears in the following classes
 java/nio/DirectByteBuffer.java
 sun/misc/Perf.java
 sun/nio/fs/NativeBuffer.java
 sun/nio/ch/IOVecWrapper.java
 sun/misc/Cleaner/ExitOnThrow.java
However none of the above overridden implementations ever create an
object in the clean() code.
 b) new PrivilegedAction created in try catch Exception block of
clean() method but for this object to be created and to be held
responsible for OOME an Exception(other than OOME) has to be thrown.

2) No new heap objects are created in the enqueue method nor
anywhere in
the deep call stack (VM.addFinalRefCount() etc) so this cannot be a
potential cause.

*Experim

Re: JDK 9 RFR of JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}

2014-01-07 Thread Lance Andersen - Oracle
looks good Joe
On Jan 7, 2014, at 6:58 PM, Joe Darcy wrote:

> Hello,
> 
> Please review the fix below to address
> 
>JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}
> 
> by a quick-and-dirty generification and deprecation of some very old classes
> 
>http://cr.openjdk.java.net/~darcy/8031369.0/
> 
> Corresponding patch below.
> 
> In the fullness of time, these classes should probably be removed from the 
> platform, but for the moment I'm more concerned with eliminating the several 
> dozen lint warnings in this code.
> 
> Thanks,
> 
> -Joe
> 
> --- old/src/share/classes/sun/misc/Cache.java2014-01-07 
> 15:51:32.0 -0800
> +++ new/src/share/classes/sun/misc/Cache.java2014-01-07 
> 15:51:32.0 -0800
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1995, 1996, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1995, 2014, Oracle and/or its affiliates. All rights 
> reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -74,8 +74,9 @@
>  * @see java.lang.Object#equals
>  * @see sun.misc.Ref
>  */
> +@Deprecated
> public
> -class Cache extends Dictionary {
> +class Cache extends Dictionary {
> /**
>  * The hash table data.
>  */
> @@ -163,7 +164,7 @@
>  * @see Cache#elements
>  * @see Enumeration
>  */
> -public synchronized Enumeration keys() {
> +public synchronized Enumeration keys() {
> return new CacheEnumerator(table, true);
> }
> 
> @@ -173,7 +174,7 @@
>  * @see Cache#keys
>  * @see Enumeration
>  */
> -public synchronized Enumeration elements() {
> +public synchronized Enumeration elements() {
> return new CacheEnumerator(table, false);
> }
> 
> @@ -305,7 +306,7 @@
>  * A Cache enumerator class.  This class should remain opaque
>  * to the client. It will use the Enumeration interface.
>  */
> -class CacheEnumerator implements Enumeration {
> +class CacheEnumerator implements Enumeration {
> boolean keys;
> int index;
> CacheEntry table[];
> --- old/src/share/classes/sun/misc/SoftCache.java2014-01-07 
> 15:51:33.0 -0800
> +++ new/src/share/classes/sun/misc/SoftCache.java2014-01-07 
> 15:51:33.0 -0800
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1998, 2006, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1998, 2014, Oracle and/or its affiliates. All rights 
> reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -101,8 +101,8 @@
>  * @see java.lang.ref.SoftReference
>  */
> 
> -
> -public class SoftCache extends AbstractMap implements Map {
> +@Deprecated
> +public class SoftCache extends AbstractMap implements 
> Map {
> 
> /* The basic idea of this implementation is to maintain an internal 
> HashMap
>that maps keys to soft references whose referents are the keys' values;
> @@ -115,18 +115,18 @@
>  */
> 
> 
> -static private class ValueCell extends SoftReference {
> +static private class ValueCell extends SoftReference {
> static private Object INVALID_KEY = new Object();
> static private int dropped = 0;
> private Object key;
> 
> -private ValueCell(Object key, Object value, ReferenceQueue queue) {
> +private ValueCell(Object key, Object value, ReferenceQueue 
> queue) {
> super(value, queue);
> this.key = key;
> }
> 
> private static ValueCell create(Object key, Object value,
> -ReferenceQueue queue)
> + ReferenceQueue queue)
> {
> if (value == null) return null;
> return new ValueCell(key, value, queue);
> @@ -154,10 +154,10 @@
> 
> 
> /* Hash table mapping keys to ValueCells */
> -private Map hash;
> +private Map hash;
> 
> /* Reference queue for cleared ValueCells */
> -private ReferenceQueue queue = new ReferenceQueue();
> +private ReferenceQueue queue = new ReferenceQueue<>();
> 
> 
> /* Process any ValueCells that have been cleared and enqueued by the
> @@ -189,7 +189,7 @@
>  *   factor is less than zero
>  */
> public SoftCache(int initialCapacity, float loadFactor) {
> -hash = new HashMap(initialCapacity, loadFactor);
> +hash = new HashMap<>(initialCapacity, loadFactor);
> }
> 
> /**
> @@ -202,7 +202,7 @@
>  *   or equal to zero
>  */
> public SoftCache(int initialCapacity) {
> -hash = new HashMap(initialCapacity);
> +hash = new HashMap<>(initialCapacity);
> }
> 
> /**
> @@ -210,7 +210,7 @@
>  * capacity and the default load factor.
>  */
> public SoftCache() {
> -hash = new HashMap();
> +hash = new HashMap<>(

RFR for JDK-8030089: java/util/zip/ZipFile/FinalizeZipFile.java intermittently fails with fastdebug builds

2014-01-07 Thread Tristan Yan

Hi All

Please help to review code fix for bug JDK-8030089.

http://cr.openjdk.java.net/~tyan/JDK-8030089/webrev.00/ 



Description:
1. Set test running on othervm mode.
2. Use infinite wait to the CountDownLatch.

Thank you
Tristan


Re: JDK 9 proposal: remove sun.misc.Ref

2014-01-07 Thread Stuart Marks

On 1/7/14 2:26 PM, Joe Darcy wrote:

public abstract class Ref {


So the type has been deprecated for at least 10 years. Rather than fixing the
warning in the class, I think the best course of action is to remove the file in
JDK 9. A build of OpenJDK without this file builds fine; if a build of the
closed sources goes fine to, I think that should be sufficient justification for
this type to be removed.

Using code search engines, there are just a handful of references to
sun.misc.Ref in the wihld, but any such uses will have several years to migrate
to the standard java.util.SoftReference before JDK 9 ships.


When in doubt, take it out.

I had a small concern about whether sun.misc.Ref still might have had some 
dependencies from the JVM (i.e. special support like the java.lang.ref classes), 
but it looks like sun.misc.Ref itself is entirely implemented in terms of 
java.lang.ref.SoftReference. I did a quick search of hotspot and I couldn't find 
any mentions of sun.misc.Ref, so I think we're clear.


s'marks


Re: JDK 9 RFR of JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}

2014-01-07 Thread Joe Darcy

Hi Mike,

On 01/07/2014 05:23 PM, Mike Duigou wrote:

Can you add the javadoc @deprecated  deprecation notice as well?


Sure; for Cache I'll point to LinkedHashMap and I'll look into finding a 
alternative to SoftCache.


Thanks for the review,

-Joe




Otherwise looks good.

Mike
On Jan 7 2014, at 15:58 , Joe Darcy  wrote:


Hello,

Please review the fix below to address

JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}

by a quick-and-dirty generification and deprecation of some very old classes

http://cr.openjdk.java.net/~darcy/8031369.0/

Corresponding patch below.

In the fullness of time, these classes should probably be removed from the 
platform, but for the moment I'm more concerned with eliminating the several 
dozen lint warnings in this code.

Thanks,

-Joe

--- old/src/share/classes/sun/misc/Cache.java2014-01-07 15:51:32.0 
-0800
+++ new/src/share/classes/sun/misc/Cache.java2014-01-07 15:51:32.0 
-0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1995, 1996, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1995, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -74,8 +74,9 @@
  * @see java.lang.Object#equals
  * @see sun.misc.Ref
  */
+@Deprecated
public
-class Cache extends Dictionary {
+class Cache extends Dictionary {
 /**
  * The hash table data.
  */
@@ -163,7 +164,7 @@
  * @see Cache#elements
  * @see Enumeration
  */
-public synchronized Enumeration keys() {
+public synchronized Enumeration keys() {
 return new CacheEnumerator(table, true);
 }

@@ -173,7 +174,7 @@
  * @see Cache#keys
  * @see Enumeration
  */
-public synchronized Enumeration elements() {
+public synchronized Enumeration elements() {
 return new CacheEnumerator(table, false);
 }

@@ -305,7 +306,7 @@
  * A Cache enumerator class.  This class should remain opaque
  * to the client. It will use the Enumeration interface.
  */
-class CacheEnumerator implements Enumeration {
+class CacheEnumerator implements Enumeration {
 boolean keys;
 int index;
 CacheEntry table[];
--- old/src/share/classes/sun/misc/SoftCache.java2014-01-07 
15:51:33.0 -0800
+++ new/src/share/classes/sun/misc/SoftCache.java2014-01-07 
15:51:33.0 -0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1998, 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -101,8 +101,8 @@
  * @see java.lang.ref.SoftReference
  */

-
-public class SoftCache extends AbstractMap implements Map {
+@Deprecated
+public class SoftCache extends AbstractMap implements Map {

 /* The basic idea of this implementation is to maintain an internal HashMap
that maps keys to soft references whose referents are the keys' values;
@@ -115,18 +115,18 @@
  */


-static private class ValueCell extends SoftReference {
+static private class ValueCell extends SoftReference {
 static private Object INVALID_KEY = new Object();
 static private int dropped = 0;
 private Object key;

-private ValueCell(Object key, Object value, ReferenceQueue queue) {
+private ValueCell(Object key, Object value, ReferenceQueue 
queue) {
 super(value, queue);
 this.key = key;
 }

 private static ValueCell create(Object key, Object value,
-ReferenceQueue queue)
+ ReferenceQueue queue)
 {
 if (value == null) return null;
 return new ValueCell(key, value, queue);
@@ -154,10 +154,10 @@


 /* Hash table mapping keys to ValueCells */
-private Map hash;
+private Map hash;

 /* Reference queue for cleared ValueCells */
-private ReferenceQueue queue = new ReferenceQueue();
+private ReferenceQueue queue = new ReferenceQueue<>();


 /* Process any ValueCells that have been cleared and enqueued by the
@@ -189,7 +189,7 @@
  *   factor is less than zero
  */
 public SoftCache(int initialCapacity, float loadFactor) {
-hash = new HashMap(initialCapacity, loadFactor);
+hash = new HashMap<>(initialCapacity, loadFactor);
 }

 /**
@@ -202,7 +202,7 @@
  *   or equal to zero
  */
 public SoftCache(int initialCapacity) {
-hash = new HashMap(initialCapacity);
+hash = new HashMap<>(initialCapacity);
 }

 /**
@@ -210,7 +210,7 @@
  * capacity and the default load factor.
  */
 public SoftCache() {
-hash = new HashMap();
+hash = new

Re: JDK 9 RFR of JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}

2014-01-07 Thread Mike Duigou
Can you add the javadoc @deprecated  deprecation notice as well?

Otherwise looks good.

Mike
On Jan 7 2014, at 15:58 , Joe Darcy  wrote:

> Hello,
> 
> Please review the fix below to address
> 
>JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}
> 
> by a quick-and-dirty generification and deprecation of some very old classes
> 
>http://cr.openjdk.java.net/~darcy/8031369.0/
> 
> Corresponding patch below.
> 
> In the fullness of time, these classes should probably be removed from the 
> platform, but for the moment I'm more concerned with eliminating the several 
> dozen lint warnings in this code.
> 
> Thanks,
> 
> -Joe
> 
> --- old/src/share/classes/sun/misc/Cache.java2014-01-07 
> 15:51:32.0 -0800
> +++ new/src/share/classes/sun/misc/Cache.java2014-01-07 
> 15:51:32.0 -0800
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1995, 1996, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1995, 2014, Oracle and/or its affiliates. All rights 
> reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -74,8 +74,9 @@
>  * @see java.lang.Object#equals
>  * @see sun.misc.Ref
>  */
> +@Deprecated
> public
> -class Cache extends Dictionary {
> +class Cache extends Dictionary {
> /**
>  * The hash table data.
>  */
> @@ -163,7 +164,7 @@
>  * @see Cache#elements
>  * @see Enumeration
>  */
> -public synchronized Enumeration keys() {
> +public synchronized Enumeration keys() {
> return new CacheEnumerator(table, true);
> }
> 
> @@ -173,7 +174,7 @@
>  * @see Cache#keys
>  * @see Enumeration
>  */
> -public synchronized Enumeration elements() {
> +public synchronized Enumeration elements() {
> return new CacheEnumerator(table, false);
> }
> 
> @@ -305,7 +306,7 @@
>  * A Cache enumerator class.  This class should remain opaque
>  * to the client. It will use the Enumeration interface.
>  */
> -class CacheEnumerator implements Enumeration {
> +class CacheEnumerator implements Enumeration {
> boolean keys;
> int index;
> CacheEntry table[];
> --- old/src/share/classes/sun/misc/SoftCache.java2014-01-07 
> 15:51:33.0 -0800
> +++ new/src/share/classes/sun/misc/SoftCache.java2014-01-07 
> 15:51:33.0 -0800
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1998, 2006, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1998, 2014, Oracle and/or its affiliates. All rights 
> reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -101,8 +101,8 @@
>  * @see java.lang.ref.SoftReference
>  */
> 
> -
> -public class SoftCache extends AbstractMap implements Map {
> +@Deprecated
> +public class SoftCache extends AbstractMap implements 
> Map {
> 
> /* The basic idea of this implementation is to maintain an internal 
> HashMap
>that maps keys to soft references whose referents are the keys' values;
> @@ -115,18 +115,18 @@
>  */
> 
> 
> -static private class ValueCell extends SoftReference {
> +static private class ValueCell extends SoftReference {
> static private Object INVALID_KEY = new Object();
> static private int dropped = 0;
> private Object key;
> 
> -private ValueCell(Object key, Object value, ReferenceQueue queue) {
> +private ValueCell(Object key, Object value, ReferenceQueue 
> queue) {
> super(value, queue);
> this.key = key;
> }
> 
> private static ValueCell create(Object key, Object value,
> -ReferenceQueue queue)
> + ReferenceQueue queue)
> {
> if (value == null) return null;
> return new ValueCell(key, value, queue);
> @@ -154,10 +154,10 @@
> 
> 
> /* Hash table mapping keys to ValueCells */
> -private Map hash;
> +private Map hash;
> 
> /* Reference queue for cleared ValueCells */
> -private ReferenceQueue queue = new ReferenceQueue();
> +private ReferenceQueue queue = new ReferenceQueue<>();
> 
> 
> /* Process any ValueCells that have been cleared and enqueued by the
> @@ -189,7 +189,7 @@
>  *   factor is less than zero
>  */
> public SoftCache(int initialCapacity, float loadFactor) {
> -hash = new HashMap(initialCapacity, loadFactor);
> +hash = new HashMap<>(initialCapacity, loadFactor);
> }
> 
> /**
> @@ -202,7 +202,7 @@
>  *   or equal to zero
>  */
> public SoftCache(int initialCapacity) {
> -hash = new HashMap(initialCapacity);
> +hash = new HashMap<>(initialCapacity);
> }
> 
> /**
> @@ -210,7 +210,7 @@
>  * capacity and the default load factor.
>  */
> public S

Re: RFR: JDK-8028726 - (prefs) Check src/solaris/native/java/util/FileSystemPreferences.c for JNI pending exceptions

2014-01-07 Thread Dan Xu

Hi All,

Thanks for your good review. I have dropped the change in 
FileSystemPreferences.java , and created the new webrev which only 
changes FileSystemPreferences.c. Please help review it. Thanks!


Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev.01/

When changing FileSystemPreferences.c, I noticed the code like 
"JNU_GetStringPlatformChars(env, java_fname, JNI_FALSE)". Because 
JNI_FALSE is passed into this function, I am wondering why our code 
still release string by calling JNU_ReleaseStringPlatformChars(env, 
java_fname, fname). Thanks!


-Dan


On 01/07/2014 01:36 AM, Alan Bateman wrote:

On 06/01/2014 22:29, Dan Xu wrote:

Hi All,

Please review the simple fix for JNI pending exceptions in 
FileSystemPreferences.c. Thanks!


Bug: https://bugs.openjdk.java.net/browse/JDK-8028726
Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev/
The update to FIleSystemPreferences.c looks okay but if I read it 
correctly then lockFile0 can never return NULL without a pending 
exception (meaning that the change to FileSystemPreferences.java could 
mask an underlying bug if it existed).


One passing comment is that this native methods could be completely 
eliminated here by changing FileSystemPreferences to lock the file via 
a FileChannel (use a FileLock as the lock handle). Also the chmod 
usage can be eliminated by mkdirs with Files.createDirectory and 
specify the permission files when creating the directory. I realize 
this is beyond the scope of what you are doing here (but an 
opportunity none the less).


-Alan




JDK 9 RFR of JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}

2014-01-07 Thread Joe Darcy

Hello,

Please review the fix below to address

JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}

by a quick-and-dirty generification and deprecation of some very old classes

http://cr.openjdk.java.net/~darcy/8031369.0/

Corresponding patch below.

In the fullness of time, these classes should probably be removed from 
the platform, but for the moment I'm more concerned with eliminating the 
several dozen lint warnings in this code.


Thanks,

-Joe

--- old/src/share/classes/sun/misc/Cache.java2014-01-07 
15:51:32.0 -0800
+++ new/src/share/classes/sun/misc/Cache.java2014-01-07 
15:51:32.0 -0800

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1995, 1996, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 1995, 2014, Oracle and/or its affiliates. All rights 
reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -74,8 +74,9 @@
  * @see java.lang.Object#equals
  * @see sun.misc.Ref
  */
+@Deprecated
 public
-class Cache extends Dictionary {
+class Cache extends Dictionary {
 /**
  * The hash table data.
  */
@@ -163,7 +164,7 @@
  * @see Cache#elements
  * @see Enumeration
  */
-public synchronized Enumeration keys() {
+public synchronized Enumeration keys() {
 return new CacheEnumerator(table, true);
 }

@@ -173,7 +174,7 @@
  * @see Cache#keys
  * @see Enumeration
  */
-public synchronized Enumeration elements() {
+public synchronized Enumeration elements() {
 return new CacheEnumerator(table, false);
 }

@@ -305,7 +306,7 @@
  * A Cache enumerator class.  This class should remain opaque
  * to the client. It will use the Enumeration interface.
  */
-class CacheEnumerator implements Enumeration {
+class CacheEnumerator implements Enumeration {
 boolean keys;
 int index;
 CacheEntry table[];
--- old/src/share/classes/sun/misc/SoftCache.java2014-01-07 
15:51:33.0 -0800
+++ new/src/share/classes/sun/misc/SoftCache.java2014-01-07 
15:51:33.0 -0800

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2006, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 1998, 2014, Oracle and/or its affiliates. All rights 
reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -101,8 +101,8 @@
  * @see java.lang.ref.SoftReference
  */

-
-public class SoftCache extends AbstractMap implements Map {
+@Deprecated
+public class SoftCache extends AbstractMap implements 
Map {


 /* The basic idea of this implementation is to maintain an 
internal HashMap
that maps keys to soft references whose referents are the keys' 
values;

@@ -115,18 +115,18 @@
  */


-static private class ValueCell extends SoftReference {
+static private class ValueCell extends SoftReference {
 static private Object INVALID_KEY = new Object();
 static private int dropped = 0;
 private Object key;

-private ValueCell(Object key, Object value, ReferenceQueue queue) {
+private ValueCell(Object key, Object value, 
ReferenceQueue queue) {

 super(value, queue);
 this.key = key;
 }

 private static ValueCell create(Object key, Object value,
-ReferenceQueue queue)
+ ReferenceQueue queue)
 {
 if (value == null) return null;
 return new ValueCell(key, value, queue);
@@ -154,10 +154,10 @@


 /* Hash table mapping keys to ValueCells */
-private Map hash;
+private Map hash;

 /* Reference queue for cleared ValueCells */
-private ReferenceQueue queue = new ReferenceQueue();
+private ReferenceQueue queue = new ReferenceQueue<>();


 /* Process any ValueCells that have been cleared and enqueued by the
@@ -189,7 +189,7 @@
  *   factor is less than zero
  */
 public SoftCache(int initialCapacity, float loadFactor) {
-hash = new HashMap(initialCapacity, loadFactor);
+hash = new HashMap<>(initialCapacity, loadFactor);
 }

 /**
@@ -202,7 +202,7 @@
  *   or equal to zero
  */
 public SoftCache(int initialCapacity) {
-hash = new HashMap(initialCapacity);
+hash = new HashMap<>(initialCapacity);
 }

 /**
@@ -210,7 +210,7 @@
  * capacity and the default load factor.
  */
 public SoftCache() {
-hash = new HashMap();
+hash = new HashMap<>();
 }


@@ -348,13 +348,13 @@
 /* Internal class for entries.
Because it uses SoftCache.this.queue, this class cannot be static.
  */
-private class Entry implements Map.Entry {
-private Map.Entry ent;
+private class Entry implements Map.Entry {
+private Ma

Re: JDK 9 RFR of raw type lint fix to java.lang.management

2014-01-07 Thread Mandy Chung


On 1/7/2014 12:30 PM, Joe Darcy wrote:

Hello,

Please review another minor lint fix of a raw type issues in the core 
libraries:


Looks good.  cc'ing serviceability-dev as java.lang.management is owned 
by the serviceability team.


Mandy


JDK 9 proposal: remove sun.misc.Ref

2014-01-07 Thread Joe Darcy

Hello,

As part of the lint clean up of core libraries, I noticed a number of 
warnings in the class sun.misc.Ref. Notable excerpts from the file:




/*
 * Copyright (c) 1995, 2004, Oracle and/or its affiliates. All rights 
reserved.

...
 * @deprecated This class has been replaced by
 * java.util.SoftReference.
 *
...
 *
 */
@Deprecated

public abstract class Ref {


So the type has been deprecated for at least 10 years. Rather than 
fixing the warning in the class, I think the best course of action is to 
remove the file in JDK 9. A build of OpenJDK without this file builds 
fine; if a build of the closed sources goes fine to, I think that should 
be sufficient justification for this type to be removed.


Using code search engines, there are just a handful of references to 
sun.misc.Ref in the wihld, but any such uses will have several years to 
migrate to the standard java.util.SoftReference before JDK 9 ships.


Comments?

-Joe


Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently

2014-01-07 Thread srikalyan chandrashekar
Hi David, TraceExceptions with fastdebug build produced some nice trace 
 . The 
native method wait(long) is where the OOME if being thrown, the deepest 
call is in


src/share/vm/gc_interface/collectedHeap.inline.hpp, line 157

--- Excerpt Begins -

147  if (!gc_overhead_limit_was_exceeded) {
148// -XX:+HeapDumpOnOutOfMemoryError and -XX:OnOutOfMemoryError support
149report_java_out_of_memory("Java heap space");
150
151if (JvmtiExport::should_post_resource_exhausted()) {
152  JvmtiExport::post_resource_exhausted(
153JVMTI_RESOURCE_EXHAUSTED_OOM_ERROR | 
JVMTI_RESOURCE_EXHAUSTED_JAVA_HEAP,
154"Java heap space");
155}
156
157THROW_OOP_0(Universe::out_of_memory_error_java_heap());
158  } else {

--- Excerpt Ends -


Would be helpful if David/some one else in the team could explain the 
latent aspects/probable cause.


---
Thanks
kalyan

On 01/06/2014 04:40 PM, David Holmes wrote:

Back from vacation ...

On 20/12/2013 4:49 PM, David Holmes wrote:

On 20/12/2013 12:57 PM, srikalyan chandrashekar wrote:
Hi David Thanks for your comments, the unguarded part(clean and 
enqueue)
in the Reference Handler thread does not seem to create any new 
objects,

so it is the application(the test in this case) which is adding objects
to heap and causing the Reference Handler to die with OOME.


The ReferenceHandler thread can only get OOME if it allocates (directly
or indirectly) - so there has to be something in the unguarded part that
causes this. Again it may be an implicit action in the VM - similar to
the class load issue for InterruptedException.


Run a debug VM with -XX:+TraceExceptions to see where the OOME is 
triggered.


David
-


David

I am still

unsure about the side effects of the code change and agree with your
thoughts(on memory exhaustion test's reliability).

PS: hotspot dev alias removed from CC.

--
Thanks
kalyan

On 12/19/13 5:08 PM, David Holmes wrote:

Hi Kalyan,

This is not a hotspot issue so I'm moving this to core-libs, please
drop hotspot from any replies.

On 20/12/2013 6:26 AM, srikalyan wrote:

Hi all,  I have been working on the bug JDK-8022321
 , this is a 
sporadic

failure and the webrev is available here
http://cr.openjdk.java.net/~srikchan/Regression/JDK-8022321_OOMEInReferenceHandler-webrev/ 






I'm really not sure what to make of this. We have a test that triggers
an out-of-memory condition but the OOME can actually turn up in the
ReferenceHandler thread causing it to terminate and the test to fail.
We previously accounted for the non-obvious occurrences of OOME due to
the Object.wait and the possible need to load the InterruptedException
class - but still the OOME can appear where we don't want it. So
finally you have just placed the whole for(;;) loop in a
try/catch(OOME) that ignores the OOME. I'm certain that makes the test
happy, but I'm not sure it is really what we want for the
ReferenceHandler thread. If the OOME occurs while cleaning, or
enqueuing then we will fail to clean and/or enqueue but there would be
no indication that has occurred and I think that is a bigger problem
than this test failing.

There may be no way to make this test 100% reliable. In fact I'd
suggest that no memory exhaustion test can be 100% reliable.

David


*
**"Root Cause:Still not known"*
2 places where there is a possibility for OOME
1) Cleaner.clean()
2) ReferenceQueue.enqueue()

1)  The cleanup code in turn has 2 places where there is potential 
for

throwing OOME,
 a) thunk Thread which is run from clean() method. This 
Runnable is

passed to Cleaner and appears in the following classes
 java/nio/DirectByteBuffer.java
 sun/misc/Perf.java
 sun/nio/fs/NativeBuffer.java
 sun/nio/ch/IOVecWrapper.java
 sun/misc/Cleaner/ExitOnThrow.java
However none of the above overridden implementations ever create an
object in the clean() code.
 b) new PrivilegedAction created in try catch Exception block of
clean() method but for this object to be created and to be held
responsible for OOME an Exception(other than OOME) has to be thrown.

2) No new heap objects are created in the enqueue method nor
anywhere in
the deep call stack (VM.addFinalRefCount() etc) so this cannot be a
potential cause.

*Experimental change to java.lang.Reference.java* :
- Put one more guard (try catch with OOME block) in the Reference
Handler Thread which may give the Reference Handler a chance to
cleanup.
This is fixing the test failure (several 1000 runs with 0 failures)
- Without the above change the test fails atleast 3-5 times for every
1000 run.

*PS*: The code change is to a very critical part of JDK and i am 
fully
not aware of the consequences of the change, hence seeking expert 
help

here. Appreciate your time and inputs towards this.







Re: JDK 9 RFR of raw type lint fix to java.lang.management

2014-01-07 Thread Lance Andersen - Oracle
+1
On Jan 7, 2014, at 3:30 PM, Joe Darcy wrote:

> Hello,
> 
> Please review another minor lint fix of a raw type issues in the core 
> libraries:
> 
>diff -r 2647b91dbc2a 
> src/share/classes/java/lang/management/ManagementFactory.java
> --- a/src/share/classes/java/lang/management/ManagementFactory.java Tue Jan 
> 07 09:58:16 2014 -0800
> +++ b/src/share/classes/java/lang/management/ManagementFactory.java Tue Jan 
> 07 12:29:22 2014 -0800
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights 
> reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -612,7 +612,7 @@
> " is not an instance of " + mxbeanInterface);
> }
> 
> -final Class[] interfaces;
> +final Class[] interfaces;
> // check if the registered MBean is a notification emitter
> boolean emitter = connection.isInstanceOf(objName, NOTIF_EMITTER);
> 
> 
> Thanks,
> 
> -Joe


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: JDK 9 RFR of raw type lint fix to java.lang.management

2014-01-07 Thread Alan Bateman

On 07/01/2014 20:30, Joe Darcy wrote:

Hello,

Please review another minor lint fix of a raw type issues in the core 
libraries:

Looks good.

-Alan.


Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently

2014-01-07 Thread srikalyan chandrashekar
Peter, getting state info out(to console or otherwise) from within 
Reference Handler's exceptions handlers have been unsuccessful.  However 
David's suggestion produced some useful trace with fast debug build and 
could get some information , see the log here 
 .


---
Thanks
kalyan

On 01/07/2014 12:42 AM, Peter Levart wrote:

On 01/07/2014 03:15 AM, srikalyan chandrashekar wrote:

Sure David will give that a try, we have so far attempted to
1. Print state data(as per the test creator peter.levart's inputs),


Hi Kalyan,

Have you been able to reproduce the OOME in that set-up? What was the 
result?


Regards, Peter


2. Use UEH(uncaught exception handler per Mandy's inputs)

--
Thanks
kalyan

On 1/6/14 4:40 PM, David Holmes wrote:

Back from vacation ...

On 20/12/2013 4:49 PM, David Holmes wrote:

On 20/12/2013 12:57 PM, srikalyan chandrashekar wrote:
Hi David Thanks for your comments, the unguarded part(clean and 
enqueue)
in the Reference Handler thread does not seem to create any new 
objects,
so it is the application(the test in this case) which is adding 
objects

to heap and causing the Reference Handler to die with OOME.


The ReferenceHandler thread can only get OOME if it allocates 
(directly
or indirectly) - so there has to be something in the unguarded part 
that

causes this. Again it may be an implicit action in the VM - similar to
the class load issue for InterruptedException.


Run a debug VM with -XX:+TraceExceptions to see where the OOME is 
triggered.


David
-


David

I am still

unsure about the side effects of the code change and agree with your
thoughts(on memory exhaustion test's reliability).

PS: hotspot dev alias removed from CC.

--
Thanks
kalyan

On 12/19/13 5:08 PM, David Holmes wrote:

Hi Kalyan,

This is not a hotspot issue so I'm moving this to core-libs, please
drop hotspot from any replies.

On 20/12/2013 6:26 AM, srikalyan wrote:

Hi all,  I have been working on the bug JDK-8022321
 , this is a 
sporadic

failure and the webrev is available here
http://cr.openjdk.java.net/~srikchan/Regression/JDK-8022321_OOMEInReferenceHandler-webrev/ 






I'm really not sure what to make of this. We have a test that 
triggers

an out-of-memory condition but the OOME can actually turn up in the
ReferenceHandler thread causing it to terminate and the test to 
fail.
We previously accounted for the non-obvious occurrences of OOME 
due to
the Object.wait and the possible need to load the 
InterruptedException

class - but still the OOME can appear where we don't want it. So
finally you have just placed the whole for(;;) loop in a
try/catch(OOME) that ignores the OOME. I'm certain that makes the 
test

happy, but I'm not sure it is really what we want for the
ReferenceHandler thread. If the OOME occurs while cleaning, or
enqueuing then we will fail to clean and/or enqueue but there 
would be

no indication that has occurred and I think that is a bigger problem
than this test failing.

There may be no way to make this test 100% reliable. In fact I'd
suggest that no memory exhaustion test can be 100% reliable.

David


*
**"Root Cause:Still not known"*
2 places where there is a possibility for OOME
1) Cleaner.clean()
2) ReferenceQueue.enqueue()

1)  The cleanup code in turn has 2 places where there is 
potential for

throwing OOME,
 a) thunk Thread which is run from clean() method. This 
Runnable is

passed to Cleaner and appears in the following classes
 java/nio/DirectByteBuffer.java
 sun/misc/Perf.java
 sun/nio/fs/NativeBuffer.java
 sun/nio/ch/IOVecWrapper.java
 sun/misc/Cleaner/ExitOnThrow.java
However none of the above overridden implementations ever create an
object in the clean() code.
 b) new PrivilegedAction created in try catch Exception 
block of

clean() method but for this object to be created and to be held
responsible for OOME an Exception(other than OOME) has to be 
thrown.


2) No new heap objects are created in the enqueue method nor
anywhere in
the deep call stack (VM.addFinalRefCount() etc) so this cannot be a
potential cause.

*Experimental change to java.lang.Reference.java* :
- Put one more guard (try catch with OOME block) in the Reference
Handler Thread which may give the Reference Handler a chance to
cleanup.
This is fixing the test failure (several 1000 runs with 0 failures)
- Without the above change the test fails atleast 3-5 times for 
every

1000 run.

*PS*: The code change is to a very critical part of JDK and i am 
fully
not aware of the consequences of the change, hence seeking 
expert help

here. Appreciate your time and inputs towards this.











Re: JDK 9 RFR of raw type lint fix to java.lang.management

2014-01-07 Thread Paul Sandoz
+1
Paul.

On Jan 7, 2014, at 9:30 PM, Joe Darcy  wrote:

> Hello,
> 
> Please review another minor lint fix of a raw type issues in the core 
> libraries:
> 
>diff -r 2647b91dbc2a 
> src/share/classes/java/lang/management/ManagementFactory.java
> --- a/src/share/classes/java/lang/management/ManagementFactory.java Tue Jan 
> 07 09:58:16 2014 -0800
> +++ b/src/share/classes/java/lang/management/ManagementFactory.java Tue Jan 
> 07 12:29:22 2014 -0800
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights 
> reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -612,7 +612,7 @@
> " is not an instance of " + mxbeanInterface);
> }
> 
> -final Class[] interfaces;
> +final Class[] interfaces;
> // check if the registered MBean is a notification emitter
> boolean emitter = connection.isInstanceOf(objName, NOTIF_EMITTER);
> 
> 
> Thanks,
> 
> -Joe



JDK 9 RFR of raw type lint fix to java.lang.management

2014-01-07 Thread Joe Darcy

Hello,

Please review another minor lint fix of a raw type issues in the core 
libraries:


diff -r 2647b91dbc2a 
src/share/classes/java/lang/management/ManagementFactory.java
--- a/src/share/classes/java/lang/management/ManagementFactory.java Tue 
Jan 07 09:58:16 2014 -0800
+++ b/src/share/classes/java/lang/management/ManagementFactory.java Tue 
Jan 07 12:29:22 2014 -0800

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights 
reserved.

  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -612,7 +612,7 @@
 " is not an instance of " + mxbeanInterface);
 }

-final Class[] interfaces;
+final Class[] interfaces;
 // check if the registered MBean is a notification emitter
 boolean emitter = connection.isInstanceOf(objName, 
NOTIF_EMITTER);



Thanks,

-Joe


hg: jdk8/tl/jdk: 8031103: java.time.Duration has wrong Javadoc Comments in toDays() and toHours()

2014-01-07 Thread roger . riggs
Changeset: 1b503dd54b95
Author:rriggs
Date:  2014-01-07 11:50 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1b503dd54b95

8031103: java.time.Duration has wrong Javadoc Comments in toDays() and toHours()
Summary: Correct specification for Duration.toDays, toHours
Reviewed-by: lancea, alanb

! src/share/classes/java/time/Duration.java



RE: Optimization in collections API

2014-01-07 Thread Jason Mehrens



Robert, If you can create a micro benchmark that fools all of the core-libs-dev 
or real world benchmark that actually shows performance improvements you might 
be able to get this patch in to the source code.
 
Previous attempts are covered under 
https://bugs.openjdk.java.net/browse/JDK-4375522 and 
https://bugs.openjdk.java.net/browse/JDK-6383986.
 
Looks like this latest version is covered under 
https://bugs.openjdk.java.net/browse/JDK-8025706.  If 8025706 arrives at the 
same conclusion as the previous attempts maybe some comments should be added to 
explain what is not present in the code.

Jason 
> From: sn...@gmx.de
> Subject: Optimization in collections API
> Date: Sun, 15 Dec 2013 11:34:01 +0100
> To: core-libs-dev@openjdk.java.net
> 
> Hello,
> 
> many real business applications make intensive use of the collections api. I 
> have spend some time and tried to improve it a bit:
> I've added a shared empty array instances to java.util.Arrays and use 
> Arrays.EMPTY_OBJECT_ARRAY where appropriate in collection classes and reduce 
> use of new java.util.Iterator instances where possible
> 
> The changes pass the existing tests in openjdk8 build (make test).
> 
> Robert
> 

  

Re: RFR for JDK-6772009 Intermittent test failure: java/util/concurrent/locks/ReentrantLock/CancelledLockLoops.java test failed with 'Completed != 2'

2014-01-07 Thread Sandeep Konchady
Hi David/Martin,

If you agree with Kalyan's fix for this issue, could one of you please sponsor 
the push.

Thanks,
Sandeep

On Dec 23, 2013, at 11:17 AM, srikalyan chandrashekar 
 wrote:

> Hi David/Martin, could any one of you sponsor this change for me?
> 
> ---
> Thanks
> kalyan
> 
> On 12/20/2013 10:28 PM, David Holmes wrote:
>> On 21/12/2013 4:19 AM, srikalyan wrote:
>>> Hi David, i retained only the changes to ITERS, ProbleMList.txt and
>>> upstream changes by Doug Lea(as pointed by Martin), could you please
>>> review the new change available here
>>> http://cr.openjdk.java.net/~srikchan/Regression/6772009-CancelledLockLoop-webrev/
>>>  
>>> .
>> 
>> Ok.
>> 
>> Thanks,
>> David
>> 
>>> -- 
>>> Thanks
>>> kalyan
>>> Ph: (408)-585-8040
>>> 
>>> 
>>> On 12/19/13, 8:10 PM, David Holmes wrote:
 Sorry Kalyan but I don't see the need for all the incidental changes
 if the primary change is to just increase the iterations. I also don't
 see why you need to do anything for BrokenBarrierException as it is
 not expected to happen and the test should just fail if it does.
 
 David
 
 On 10/12/2013 6:15 AM, srikalyan wrote:
> Hi David/Martin a gentle reminder for review.
> 
> -- 
> Thanks
> kalyan
> Ph: (408)-585-8040
> 
> 
> On 12/2/13, 11:21 AM, srikalyan wrote:
>> Hi David, Thanks for the review, the new webrev is hosted at
>> http://cr.openjdk.java.net/~cl/host_for_kal/6772009-CancelledLockLoop/ 
>> . Please see inline text.
>> 
>> On 11/20/13, 6:35 PM, David Holmes wrote:
>>> On 21/11/2013 10:28 AM, Martin Buchholz wrote:
 I again tried and failed to reproduce a failure.  Even if I go whole
 hog
 and multiply TIMEOUT by 100 and divide ITERS by 100, the test
 continues to
 PASS.  Is it just me?!
>>> 
>>> I think you are going the wrong way Martin - you want the timeout to
>>> be smaller than the time it takes to execute ITERS.
>>> 
 I don't think there's any reason to make result long.  It's not even
 used
 except to inhibit hotspot optimizations.
 
 +private volatile long result = 17;//Can get int overflow,so
 using long
>>> 
>>> Further the subsequent use of += is incorrect as it is not an atomic
>>> operation. Even if we don't care about the value, it looks bad.
>> Made the necessary changes for atomic update.
>>> 
>>> I'm not sure result must be updated if we get a
>>> BrokenBarrierException either. Probably harmless, but necessary?
>> I retained it in the fix for completeness in updating the numbers,
>> please let me know if you still think otherwise.
>>> 
 need to fix spelling and spacing below.
 
 +barrier.await();//If a BrokeBarrierException happens
 here(due to
>>> 
>>> There are a number of style issues with spacing around comments.
>> Fixed the spelling error and styling issues.
>>> 
>>> And I don't think this change is sufficient to claim co-author status
>>> with Doug either ;-)
>> Removed the claim :)
>>> 
>>> The additional tracing may be useful but seems stylistically
>>> different from the rest of the code.
>> Retained the tracking to understand if it is again the timing issue
>> which is the cause in an event of a failure, however i can remove it
>> if you think it is not necessary (OR) include an alternate solution as
>> you may want to suggest.
>>> 
>>> Overall I'm suspicious that the changed timeout actually fixes
>>> anything - normally we need to add longer timeouts not shorter ones.
>>> Does this fail on a range of machines or only specific ones? Have we
>>> verified that the clocks/timers are behaving properly on those
>>> systems?
>> Here the time out is not about waiting for threads to complete
>> something but to "be interrupted" before being considered done, so we
>> decreased the timeout. However we now chose to increase the number of
>> iterations to 500 from 100(thanks to tristan for the
>> suggestion) instead of decreasing the timeout as done earlier because
>> the increasing iterations ensures the threads are busy for long time
>> curtailing the need to touch the timeout.
>> 
>>> 
>>> Thanks,
>>> David
>> 
>> -- 
>> Thanks
>> kalyan
>> Ph: (408)-585-8040
>> 
>>> 
 
 
 
 On Wed, Nov 20, 2013 at 11:52 AM, srikalyan <
 srikalyan.chandrashe...@oracle.com> wrote:
 
>  Hi Martin , apologies for the delay , was trying to get help for
> hosting
> my webrev.  .  Please see inline text.
> 
> 
> On 11/19/13, 10:35 PM, Martin Buchholz wrote:
> 
> Hi Kalyan,
> 
>  None of us can review your changes yet because you ha

Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-07 Thread David Holmes

On 7/01/2014 8:36 PM, Tristan Yan wrote:

Hi David
You're totally right. Sorry I ask you review it again.

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.02/


Looks good now.

Thanks,
David


Thank you very much.
Tristan

On 01/07/2014 05:18 PM, David Holmes wrote:

On 7/01/2014 6:16 PM, Tristan Yan wrote:

Thank you, David
I fixed copyright and change back sleep.
println was intended to be left in. This test was failed with timeout,
printf could help us to detect the value of total_turns_taken and
expected_turns_taken.
Please review it again

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.01/


Comma after 2014 still missing in copyright.

You need to read total_turns_taken.get() once and use that value in
both the println and the == test, so that you print the value you
actually tested.

David


Regards
Tristan

On 01/07/2014 03:10 PM, David Holmes wrote:

Hi Tristan,

On 7/01/2014 4:43 PM, Tristan Yan wrote:

Hi All
Please help to review the code change for JDK-7027502.

http://cr.openjdk.java.net/~tyan/JDK-7027502/

Description:
This test was failed in JPRT test but recently we test with same
binaries run, it doesn't fail any more. The intention for this code
change is bringing this test back to normal test and make this test
robust and more informative.  Change includes
1. Remove this test from ProblemList.
2. Change static member total_turns_taken into a local variable.

Please let me know your comment on this.


   2  * Copyright (c) 2004,2014 Oracle and/or its affiliates. All
rights reserved.

Correct copyright format should have a space before 2014 and a comma
after:

 * Copyright (c) 2004, 2014, Oracle and/or its affiliates. All rights
reserved.

---

Was this println intended to be left in?

 114 System.out.println("total_turns_taken =" +
total_turns_taken +
 115 ";expected_turns_taken =" +
expected_turns_taken);
 116 if ( total_turns_taken.get() ==
expected_turns_taken ) {


You only want to read total_turns_taken once otherwise you may see
misleading print outs.

---

119 /* Create some monitor contention by sleeping with
lock */
 120 if ( default_contention_sleep > 0 ) {
 121 System.out.println("Context sleeping, to
create contention");
 122 try {
 123 turn.wait((long)
default_contention_sleep);
 124 } catch (InterruptedException ignore) { }
 125 }

By changing the Thread.sleep to turn.wait you no longer introduce any
contention as the wait() will release the monitor.

David


Thank you.
Tristan






Re: JDK 9 RFR - 8031142 [was: Re: Using @implSpec in AbstractMap, AbstractCollection, AbstractList, etc]

2014-01-07 Thread Chris Hegarty
On 6 Jan 2014, at 21:19, Martin Buchholz  wrote:

> Your change looks good, except there's one more trailing  to remove.

Thanks for the review. I found the trailing  and removed it.

-Chris.



Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-07 Thread Tristan Yan

Hi David
You're totally right. Sorry I ask you review it again.

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.02/

Thank you very much.
Tristan

On 01/07/2014 05:18 PM, David Holmes wrote:

On 7/01/2014 6:16 PM, Tristan Yan wrote:

Thank you, David
I fixed copyright and change back sleep.
println was intended to be left in. This test was failed with timeout,
printf could help us to detect the value of total_turns_taken and
expected_turns_taken.
Please review it again

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.01/


Comma after 2014 still missing in copyright.

You need to read total_turns_taken.get() once and use that value in 
both the println and the == test, so that you print the value you 
actually tested.


David


Regards
Tristan

On 01/07/2014 03:10 PM, David Holmes wrote:

Hi Tristan,

On 7/01/2014 4:43 PM, Tristan Yan wrote:

Hi All
Please help to review the code change for JDK-7027502.

http://cr.openjdk.java.net/~tyan/JDK-7027502/

Description:
This test was failed in JPRT test but recently we test with same
binaries run, it doesn't fail any more. The intention for this code
change is bringing this test back to normal test and make this test
robust and more informative.  Change includes
1. Remove this test from ProblemList.
2. Change static member total_turns_taken into a local variable.

Please let me know your comment on this.


   2  * Copyright (c) 2004,2014 Oracle and/or its affiliates. All
rights reserved.

Correct copyright format should have a space before 2014 and a comma
after:

 * Copyright (c) 2004, 2014, Oracle and/or its affiliates. All rights
reserved.

---

Was this println intended to be left in?

 114 System.out.println("total_turns_taken =" +
total_turns_taken +
 115 ";expected_turns_taken =" +
expected_turns_taken);
 116 if ( total_turns_taken.get() ==
expected_turns_taken ) {


You only want to read total_turns_taken once otherwise you may see
misleading print outs.

---

119 /* Create some monitor contention by sleeping with
lock */
 120 if ( default_contention_sleep > 0 ) {
 121 System.out.println("Context sleeping, to
create contention");
 122 try {
 123 turn.wait((long) 
default_contention_sleep);

 124 } catch (InterruptedException ignore) { }
 125 }

By changing the Thread.sleep to turn.wait you no longer introduce any
contention as the wait() will release the monitor.

David


Thank you.
Tristan






Re: RFR java.time.Duration spec correction (8031103)

2014-01-07 Thread Alan Bateman

On 06/01/2014 19:09, roger riggs wrote:
Please review this minor specification correction to the 
java.time.Duration.toDays() and
toHours() methods.  Only the javadoc is corrected, no code or tests 
are affected.


Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-duration-javadoc-8031103/

This looks okay to me (a cut&paste error I assume).

-Alan


Re: Theoretical data race on java.util.logging.Handler.sealed

2014-01-07 Thread Peter Levart

Hi Mandy, Daniel,

Thanks for reviews. I just pushed this change to jdk9-dev/jdk ...

Regards, Peter

On 12/23/2013 05:50 AM, Mandy Chung wrote:


On 12/22/2013 5:23 AM, Peter Levart wrote:

Hi Mandy,

On 12/19/2013 10:38 PM, Mandy Chung wrote:

On 12/19/13 7:49 AM, Peter Levart wrote:

Hi Mandy, Daniel,

I didn't like the package-protected getters either. So here's 
another variant that replaces Handler.configure() method with a 
package-protected constructor which is chained from JDK subclasses:


http://cr.openjdk.java.net/~plevart/jdk8-tl/jul.Handler.sealed/webrev.07/ 





Looks good.  Thanks for making the change and the new test. It'd be 
good to close the handlers by the test. The test is running in 
othervm mode and the Cleaner thread will close the handler when VM 
exits and the test is fine as it is.


Well, not really. The Cleaner only closes Handlers that are attached 
to Loggers but the test just instantiates Handlers and doesn't add 
them to any Loggers. It's harmless as it is, othervm will exit 
nevertheless and resources will be freed...


I tried closing Handlers at the end of test, but that requires 
"control" LoggingPermission and we don't want to run the test with 
"control" permission since we want to check that instantiating 
Handlers (SocketHandler too) doesn't require "control" permission.


Thanks and the test is fine as it is.



So should anything else be done before pushing this to jdk9/dev ?


Fix looks good and have a regression test.  It's good to go and push 
to jdk9/dev.No other approval needed.


thanks
Mandy




Re: RFR: JDK-8028726 - (prefs) Check src/solaris/native/java/util/FileSystemPreferences.c for JNI pending exceptions

2014-01-07 Thread Alan Bateman

On 06/01/2014 22:29, Dan Xu wrote:

Hi All,

Please review the simple fix for JNI pending exceptions in 
FileSystemPreferences.c. Thanks!


Bug: https://bugs.openjdk.java.net/browse/JDK-8028726
Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev/
The update to FIleSystemPreferences.c looks okay but if I read it 
correctly then lockFile0 can never return NULL without a pending 
exception (meaning that the change to FileSystemPreferences.java could 
mask an underlying bug if it existed).


One passing comment is that this native methods could be completely 
eliminated here by changing FileSystemPreferences to lock the file via a 
FileChannel (use a FileLock as the lock handle). Also the chmod usage 
can be eliminated by mkdirs with Files.createDirectory and specify the 
permission files when creating the directory. I realize this is beyond 
the scope of what you are doing here (but an opportunity none the less).


-Alan


Re: RFR: JDK-8028726 - (prefs) Check src/solaris/native/java/util/FileSystemPreferences.c for JNI pending exceptions

2014-01-07 Thread Chris Hegarty
On 6 Jan 2014, at 22:29, Dan Xu  wrote:

> Hi All,
> 
> Please review the simple fix for JNI pending exceptions in 
> FileSystemPreferences.c. Thanks!
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8028726
> Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev/

Looks good to me Dan.

Trivially, I don’t think you need the "if (result != null) {“. If the native 
method “fails” and returns NULL, there will be a pending exception which will 
be thrown automatically when transitioning back to Java-land. But, what you 
have is arguably more robust., so thumbs up from me.

-Chris.

> -Dan



Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-07 Thread David Holmes

On 7/01/2014 6:16 PM, Tristan Yan wrote:

Thank you, David
I fixed copyright and change back sleep.
println was intended to be left in. This test was failed with timeout,
printf could help us to detect the value of total_turns_taken and
expected_turns_taken.
Please review it again

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.01/


Comma after 2014 still missing in copyright.

You need to read total_turns_taken.get() once and use that value in both 
the println and the == test, so that you print the value you actually 
tested.


David


Regards
Tristan

On 01/07/2014 03:10 PM, David Holmes wrote:

Hi Tristan,

On 7/01/2014 4:43 PM, Tristan Yan wrote:

Hi All
Please help to review the code change for JDK-7027502.

http://cr.openjdk.java.net/~tyan/JDK-7027502/

Description:
This test was failed in JPRT test but recently we test with same
binaries run, it doesn't fail any more. The intention for this code
change is bringing this test back to normal test and make this test
robust and more informative.  Change includes
1. Remove this test from ProblemList.
2. Change static member total_turns_taken into a local variable.

Please let me know your comment on this.


   2  * Copyright (c) 2004,2014 Oracle and/or its affiliates. All
rights reserved.

Correct copyright format should have a space before 2014 and a comma
after:

 * Copyright (c) 2004, 2014, Oracle and/or its affiliates. All rights
reserved.

---

Was this println intended to be left in?

 114 System.out.println("total_turns_taken =" +
total_turns_taken +
 115 ";expected_turns_taken =" +
expected_turns_taken);
 116 if ( total_turns_taken.get() ==
expected_turns_taken ) {


You only want to read total_turns_taken once otherwise you may see
misleading print outs.

---

119 /* Create some monitor contention by sleeping with
lock */
 120 if ( default_contention_sleep > 0 ) {
 121 System.out.println("Context sleeping, to
create contention");
 122 try {
 123 turn.wait((long) default_contention_sleep);
 124 } catch (InterruptedException ignore) { }
 125 }

By changing the Thread.sleep to turn.wait you no longer introduce any
contention as the wait() will release the monitor.

David


Thank you.
Tristan




Re: ObjectIn/OutputStream improvements

2014-01-07 Thread Chris Hegarty
On 15 Dec 2013, at 10:29, Robert Stupp  wrote:

> Hi,
> 
> I digged through the object serialization code and found some lines that 
> could be optimized to reduce the number of calls to System.arraycopy() and 
> temporary object allocations especially during string (de)serialization.
> In short sentences the changes are:
> ObjectInputStream:
> - skip primitive/object reading if no primitive/objects in class 
> (defaultReadFields method)
> - use shared StringBuilder for string reading (prevent superfluous object 
> allocations of one StingBuilder and one implicit char[] for each string being 
> deserialized)
> ObjectOutputStream:
> - skip primitive/object writing if no primitive/objects in class 
> (defaultWriteFields method)
> - use unsafe access to calculate UTF-length
> - use unsafe access in readBytes() and writeChars() methods to access String 
> value field
> - removed cbuf field
> ObjectStreamClass/ObjectStreamField:
> - minor improvement in getClassSignature ; share method code with 
> ObjectStreamField (return constant string for primitives)
> 
> I have tested the changes in a big Java installation at a customer 
> (backported the Java8 ObjectIn/OutputStream including the changes to Java6) 
> and a long running real application performance test resulted in reduced CPU 
> usage (from about 60% per server to 50%).
> The changes I made in openjdk8 pass all tests.
> 
> Since I have no experience how to contribute code to openjdk in form of a 
> push/changeset I have added the diff (hg export -g) to this email.

Did you attach the diffs? I don’t see any attachment, it may be that the 
attachment was stripped. Can you try resending inline?

You should take a look at the OpenJDK How to Contribute page [1]. Paying 
particular attention to the OCA, without it we will not be able to move your 
patch forward. 

Thanks,
-Chris.

[1] http://openjdk.java.net/contribute/

> 
> Robert
> 
> 



Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently

2014-01-07 Thread Peter Levart

On 01/07/2014 03:15 AM, srikalyan chandrashekar wrote:

Sure David will give that a try, we have so far attempted to
1. Print state data(as per the test creator peter.levart's inputs),


Hi Kalyan,

Have you been able to reproduce the OOME in that set-up? What was the 
result?


Regards, Peter


2. Use UEH(uncaught exception handler per Mandy's inputs)

--
Thanks
kalyan

On 1/6/14 4:40 PM, David Holmes wrote:

Back from vacation ...

On 20/12/2013 4:49 PM, David Holmes wrote:

On 20/12/2013 12:57 PM, srikalyan chandrashekar wrote:
Hi David Thanks for your comments, the unguarded part(clean and 
enqueue)
in the Reference Handler thread does not seem to create any new 
objects,
so it is the application(the test in this case) which is adding 
objects

to heap and causing the Reference Handler to die with OOME.


The ReferenceHandler thread can only get OOME if it allocates (directly
or indirectly) - so there has to be something in the unguarded part 
that

causes this. Again it may be an implicit action in the VM - similar to
the class load issue for InterruptedException.


Run a debug VM with -XX:+TraceExceptions to see where the OOME is 
triggered.


David
-


David

I am still

unsure about the side effects of the code change and agree with your
thoughts(on memory exhaustion test's reliability).

PS: hotspot dev alias removed from CC.

--
Thanks
kalyan

On 12/19/13 5:08 PM, David Holmes wrote:

Hi Kalyan,

This is not a hotspot issue so I'm moving this to core-libs, please
drop hotspot from any replies.

On 20/12/2013 6:26 AM, srikalyan wrote:

Hi all,  I have been working on the bug JDK-8022321
 , this is a 
sporadic

failure and the webrev is available here
http://cr.openjdk.java.net/~srikchan/Regression/JDK-8022321_OOMEInReferenceHandler-webrev/ 






I'm really not sure what to make of this. We have a test that 
triggers

an out-of-memory condition but the OOME can actually turn up in the
ReferenceHandler thread causing it to terminate and the test to fail.
We previously accounted for the non-obvious occurrences of OOME 
due to
the Object.wait and the possible need to load the 
InterruptedException

class - but still the OOME can appear where we don't want it. So
finally you have just placed the whole for(;;) loop in a
try/catch(OOME) that ignores the OOME. I'm certain that makes the 
test

happy, but I'm not sure it is really what we want for the
ReferenceHandler thread. If the OOME occurs while cleaning, or
enqueuing then we will fail to clean and/or enqueue but there 
would be

no indication that has occurred and I think that is a bigger problem
than this test failing.

There may be no way to make this test 100% reliable. In fact I'd
suggest that no memory exhaustion test can be 100% reliable.

David


*
**"Root Cause:Still not known"*
2 places where there is a possibility for OOME
1) Cleaner.clean()
2) ReferenceQueue.enqueue()

1)  The cleanup code in turn has 2 places where there is 
potential for

throwing OOME,
 a) thunk Thread which is run from clean() method. This 
Runnable is

passed to Cleaner and appears in the following classes
 java/nio/DirectByteBuffer.java
 sun/misc/Perf.java
 sun/nio/fs/NativeBuffer.java
 sun/nio/ch/IOVecWrapper.java
 sun/misc/Cleaner/ExitOnThrow.java
However none of the above overridden implementations ever create an
object in the clean() code.
 b) new PrivilegedAction created in try catch Exception block of
clean() method but for this object to be created and to be held
responsible for OOME an Exception(other than OOME) has to be thrown.

2) No new heap objects are created in the enqueue method nor
anywhere in
the deep call stack (VM.addFinalRefCount() etc) so this cannot be a
potential cause.

*Experimental change to java.lang.Reference.java* :
- Put one more guard (try catch with OOME block) in the Reference
Handler Thread which may give the Reference Handler a chance to
cleanup.
This is fixing the test failure (several 1000 runs with 0 failures)
- Without the above change the test fails atleast 3-5 times for 
every

1000 run.

*PS*: The code change is to a very critical part of JDK and i am 
fully
not aware of the consequences of the change, hence seeking expert 
help

here. Appreciate your time and inputs towards this.









Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-07 Thread Tristan Yan

Thank you, David
I fixed copyright and change back sleep.
println was intended to be left in. This test was failed with timeout, 
printf could help us to detect the value of total_turns_taken and 
expected_turns_taken.

Please review it again

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.01/

Regards
Tristan

On 01/07/2014 03:10 PM, David Holmes wrote:

Hi Tristan,

On 7/01/2014 4:43 PM, Tristan Yan wrote:

Hi All
Please help to review the code change for JDK-7027502.

http://cr.openjdk.java.net/~tyan/JDK-7027502/

Description:
This test was failed in JPRT test but recently we test with same
binaries run, it doesn't fail any more. The intention for this code
change is bringing this test back to normal test and make this test
robust and more informative.  Change includes
1. Remove this test from ProblemList.
2. Change static member total_turns_taken into a local variable.

Please let me know your comment on this.


   2  * Copyright (c) 2004,2014 Oracle and/or its affiliates. All 
rights reserved.


Correct copyright format should have a space before 2014 and a comma 
after:


 * Copyright (c) 2004, 2014, Oracle and/or its affiliates. All rights 
reserved.


---

Was this println intended to be left in?

 114 System.out.println("total_turns_taken =" + 
total_turns_taken +
 115 ";expected_turns_taken =" + 
expected_turns_taken);
 116 if ( total_turns_taken.get() == 
expected_turns_taken ) {



You only want to read total_turns_taken once otherwise you may see 
misleading print outs.


---

119 /* Create some monitor contention by sleeping with 
lock */

 120 if ( default_contention_sleep > 0 ) {
 121 System.out.println("Context sleeping, to 
create contention");

 122 try {
 123 turn.wait((long) default_contention_sleep);
 124 } catch (InterruptedException ignore) { }
 125 }

By changing the Thread.sleep to turn.wait you no longer introduce any 
contention as the wait() will release the monitor.


David


Thank you.
Tristan