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

[Chris Hegarty]

Hi Kris,

I think this change should be fine, but I would like Alan to comment also.

Thanks,
-Chris.

On 11/18/11 10:44 AM, Chris Hegarty wrote:
> 
> 
> -------- 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.
>>
>>
>> ±¾µçÓÊ(°üÀ¨Èκθ½¼þ)¿ÉÄܺ¬ÓлúÃÜ×ÊÁϲ¢ÊÜ·¨Âɱ£»¤¡£ÈçÄú²»ÊÇÕýÈ·µÄÊÕ¼þÈË£¬ÇëÄúÁ¢¼´É¾³ý±¾Óʼþ¡£Çë²»Òª½«±¾µçÓʽøÐи´ÖƲ¢ÓÃ×÷ÈκÎÆäËûÓÃ;¡¢»ò͸¶±¾ÓʼþÖ®ÄÚÈÝ¡£Ð»Ð»¡£
>>