Fwd: Re: Suppress creation of SocksSocketImpl in SocketAdaptor's constructor

[Chris Hegarty]

-------- Original Message --------
Subject: Re: Suppress creation of SocksSocketImpl in SocketAdaptor's
constructor
Date: Fri, 18 Nov 2011 16:38:28 +0800
From: Krystal Mok <rednaxelafx at gmail.com>
To: ÈöåÈ <sajia at taobao.com>
CC: jdk8-dev at openjdk.java.net <jdk8-dev at openjdk.java.net>

Oops, looks like my company email stripped the attachment.
Here's that patch again:

diff -r 00e2c88e2234 src/share/classes/sun/nio/ch/SocketAdaptor.java
--- a/src/share/classes/sun/nio/ch/SocketAdaptor.java   Thu Nov 17 10:46:02
2011 -0800
+++ b/src/share/classes/sun/nio/ch/SocketAdaptor.java   Fri Nov 18 15:37:20
2011 +0800
@@ -57,13 +57,17 @@
     // 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 RuntimeException("should not reach here");
+        }
     }

     public SocketChannel getChannel() {


- Kris

2011/11/18 ÈöåÈ <sajia at taobao.com>

> Hi all,
>
> 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.
>
> The attached patch is to get rid of creation of the useless
> SocksSocketImpl.
>
> 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.
>
> I don't have a bug ID for this yet. Could anyone kindly open a bug for
> this?
>
> Regards,
> Kris Mok
> Software Engineer, Taobao (http://www.taobao.com)
>
> ________________________________
> This email (including any attachments) is confidential and may be legally
> privileged. If you received this email in error, please delete it
> immediately and do not copy it or use it for any purpose or disclose its
> contents to any other person. Thank you.
>
>
> ±¾µçÓÊ(°üÀ¨Èκθ½¼þ)¿ÉÄܺ¬ÓлúÃÜ×ÊÁϲ¢ÊÜ·¨Âɱ£»¤¡£ÈçÄú²»ÊÇÕýÈ·µÄÊÕ¼þÈË£¬ÇëÄúÁ¢¼´É¾³ý±¾Óʼþ¡£Çë²»Òª½«±¾µçÓʽøÐи´ÖƲ¢ÓÃ×÷ÈκÎÆäËûÓÃ;¡¢»ò͸¶±¾ÓʼþÖ®ÄÚÈÝ¡£Ð»Ð»¡£
>