United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-7115586 (so) Suppress creation of SocketImpl in SocketAdaptor's constructor
JDK-7115586 : (so) Suppress creation of SocketImpl in SocketAdaptor's constructor

Details
Type:
Bug
Submit Date:
2011-11-25
Status:
Closed
Updated Date:
2013-04-20
Project Name:
JDK
Resolved Date:
2012-05-13
Component:
core-libs
OS:
generic
Sub-Component:
java.nio
CPU:
generic
Priority:
P4
Resolution:
Fixed
Affected Versions:
7
Fixed Versions:

Related Reports
Backport:
Backport:
Backport:

Sub Tasks

Description
I'd like to propose the following change to JDK's sun.nio.ch.SocketAdaptor:

In OpenJDK 6, 7 and 8, sun.nio.ch.SocketAdaptor is used to adapt a SocketChannelImpl to a java.net.Socket.

A comment in the constructor of SocketAdaptor says "super will create a useless impl", but it actually doesn't have to. AFAICT the SocksSocketImpl instance created here really isn't used at all in SocketAdaptor, unless someone invokes non-public methods on java.net.Socket via reflection.

At a glance, creating a useless SocksSocketImpl for every SocketAdaptor seems innocent. But that's not the case when the allocation rate/heap memory pressure is high. SocksSocketImpl has a finalizer (inherited from PlainSocketImpl), and when:
 * old generation is large
 * there are a lot of ready-to-die SocksSocketImpl instances
these instances can be queued up in the finalizer queue, which increases the heap pressure.
 
In one of our Hadoop NameNode nodes in production, excessive SocksSocketImpl instances had been causing frequent CMS collections + long reference processing pause. Using this patch helps side-steping the problem.

see futher discussion on the nio-dev mailing list:
  http://mail.openjdk.java.net/pipermail/nio-dev/2011-November/001479.html

                                    

Comments
SUGGESTED FIX

diff -r 2db942c7eb9c src/share/classes/sun/nio/ch/SocketAdaptor.java
--- a/src/share/classes/sun/nio/ch/SocketAdaptor.java   Mon Nov 21 12:57:36 2011 +0000
+++ b/src/share/classes/sun/nio/ch/SocketAdaptor.java   Fri Nov 25 11:33:47 2011 +0000
@@ -57,13 +57,17 @@ public class SocketAdaptor
     // Timeout "option" value for reads
     private volatile int timeout = 0;

-    // ## super will create a useless impl
-    private SocketAdaptor(SocketChannelImpl sc) {
+    private SocketAdaptor(SocketChannelImpl sc) throws SocketException {
+        super((SocketImpl) null);
         this.sc = sc;
     }

     public static Socket create(SocketChannelImpl sc) {
-        return new SocketAdaptor(sc);
+        try {
+            return new SocketAdaptor(sc);
+        } catch (SocketException e) {
+            throw new InternalError("Should not reach here");
+        }
     }

     public SocketChannel getChannel() {
                                     
2011-11-25
EVALUATION

A good idea.
                                     
2011-11-25
PUBLIC COMMENTS

Newer code can invoke bind and set/getOption directly, no need to use the socket() method anymore.
                                     
2011-11-25
EVALUATION

http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e5ecbf555679
                                     
2011-11-25



Hardware and Software, Engineered to Work Together