adding rsockets support into JDK
Lu, Yingqi
yingqi.lu at intel.com
Tue Jul 3 17:46:58 UTC 2018
Hi Alan,
Thank you for the feedback. I will modify the patch based on your current feedback.
Thanks,
Lucy
>-----Original Message-----
>From: Alan Bateman [mailto:Alan.Bateman at oracle.com]
>Sent: Tuesday, July 3, 2018 10:43 AM
>To: Lu, Yingqi <yingqi.lu at intel.com>
>Cc: nio-dev at openjdk.java.net; Viswanathan, Sandhya
><sandhya.viswanathan at intel.com>; Aundhe, Shirish <shirish.aundhe at intel.com>;
>Kaczmarek, Eric <eric.kaczmarek at intel.com>
>Subject: Re: adding rsockets support into JDK
>
>On 01/07/2018 04:01, Lu, Yingqi wrote:
>> Hi All,
>>
>> Here is the webrev.08 of the patch
>> http://cr.openjdk.java.net/~ylu/8195160.08/
>>
>> In this version,
>>
>> 1. Added Javadoc for 5 newly introduced APIs inside jdk.net.Sockets. Thanks to
>Alan for his help!
>>
>> 2. Changed RQSIZE, SQSIZE and INLINE constants from public and private;
>Added comments for RDMA_SQSIZE, RDMA_RQSIZE and RDMA_INLINE options.
>>
>> 3. Currently, the patch compiles on Linux, Windows and MacOS.
>>
>I did another pass over the API changes.
>
>In SocketImpl, the setSocket and setServerSocket methods are changed to
>protected so this means this will be part of the SocketImpl API. Is this needed?
>(I'm just wondering if we can avoid this API change).
>
>The API addition to ServerSocket is okay, we'll just need to add the @since when
>it gets targeted.
>
>Thanks for including the javadoc for the methods added to jdk.net.Sockets, I think
>that looks good now (just one place where it has @since N).
>
>For RdmaSocketOptions then one suggestion is to look at
>java.net.StandardSocketOptions or jdk.net.ExtendedSocketOptions to see the
>style used for the other socket options. I think the main thing that is missing is
>text to give some idea of the values allowed and some idea on when the socket
>option can be set (before the socket it bound, after it is bound, only when
>connected).
>
>One comment on the implementation for now is that I see
>supportedOptions() is checking if the impl is the RDMA impl. I assume this isn't
>needed as it delegates to the impl. My point is that I don't think Socket and
>ServerSocket should have code checking if the impl class is
>"rdma.ch.RdmaSocketImpl".
>
>I will go through the implementation and tests in more detail once I can find time.
>
>-Alan
More information about the nio-dev
mailing list