adding rsockets support into JDK

[Lu, Yingqi]

Hi Alan,

Thank you very much for your feedback!

I agree that jdk.net.RdmaSocketImplFactory is not necessary anymore. I will remove it in next version.

I will remove the inheritance from SocketChannelImpl/ServerSocketChannelImpl, replace them with SocketChannel/ServerSocketChannel. My initial thought was to avoid the code duplication, but I totally agree with you on your concern for future maintenance. 

I will also make other changes following your suggestions and start to add test cases. I will start from the examples of regular sockets and socket channels and add ones specific to RDMA.

Thank you!!
Lucy

>-----Original Message-----
>From: Alan Bateman [mailto:Alan.Bateman at oracle.com]
>Sent: Friday, May 11, 2018 5:18 AM
>To: Lu, Yingqi <yingqi.lu at intel.com>; nio-dev at openjdk.java.net
>Cc: 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 04/05/2018 17:42, Lu, Yingqi wrote:
>> Hi All,
>>
>> The version 3 of the patch is available now at
>> http://cr.openjdk.java.net/~ylu/8195160.03/
>>
>> In this version, I have changed following items:
>>
>> 1. Added Sockets.openRdmaSelector() function
>>
>> 2. Replaced "changing system-wide SocketImplFactory" with returning
>Socket(impl). I did the similar change to ServerSocket too. I also moved
>RdmaSocketImplFactory from jdk.net to rdma.ch.
>>
>> 3. Reduced number the factory methods to 2, openRdmaSocket() and
>openRdmaServerSocket().
>>
>> 4. Removed the changes to InetAddress.
>>
>> 5. Removed the methods changed to public in Socket/ServerSocket
>>
>> Please let me know your feedback and comments.
>>
>I looked through the current version.
>
>The API additions to jdk.net.Sockets API look good. The javadoc will need to
>be fleshed out of course.
>
>Did you mean to include jdk.net.RdmaSocketImplFactory in the API? The
>factory methods can create the RdmaSocketImpl directly so it may not be
>needed.
>
>jdk.net.RdmaSocketOptions looks right. It will of course need javadoc and
>some refactoring so that hardcoded values aren't in shared code.
>
>There are a few changes to the java.net API that will need consideration.
>Having a protected constructor that takes a SocketImpl looks reasonable on
>first glance it is consistent with the protected constructor in Socket.
>
>The addition of protected setters to SocketImpl may be okay too although not
>strictly needed as the fd and address fields are protected so they can be set
>by implementations already.
>
>You've changed ServerSocket accept to support the RdmaSocketImpl but I
>don't think you need that. Instead, the ServerSocket returned by
>openServerSocket can override the accept method. I think this means you can
>drop the qualified export of jdk.net to java.base too.
>
>I'm a bit uncomfortable with extending SocketChannelImpl and
>ServerSocketChannelImpl as proposed because it tightly ties the RMDA
>implementation in jdk.net. It means we can't change anything in say
>SocketChannelImpl without breaking the RDMA implementation. Given that
>the RMDA implementations override almost everything then maybe they
>should just extend SocketChannel directly and we'll need to the duplication as
>needed.
>
>The extending of PollSelectorImpl is probably okay, just needs a comment to
>poll method to explain what it is protected.
>
>libextnet also builds on Solaris so the updates to Lib-jdk.net.gmk will need a
>few adjustments to ensure that it continues to build (you'll need to add -
>lrdmacm to LIBS_linux rather than LIBS for example).
>
>Are you planning to develop tests for this? We also have the issue how this is
>going to be tested and maintained over the long term.
>
>-Alan