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<rednaxel...@gmail.com>
> To: 撒迦<sa...@taobao.com>
> CC: jdk8-...@openjdk.java.net<jdk8-...@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 撒迦<sa...@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.
>>
>>
>> 本电邮(包括任何附件)可能含有机密资料并受法律保护。如您不是正确的收件人，请您立即删除本邮件。请不要将本电邮进行复制并用作任何其他用途、或透露本邮件之内容。谢谢。
>>
