adding rsockets support into JDK
Lu, Yingqi
yingqi.lu at intel.com
Fri Nov 16 17:28:06 UTC 2018
Hi Chris,
I have updated the CSR https://bugs.openjdk.java.net/browse/JDK-8205186
1. I added the link to Javadoc files for jdk.net
2. I also zip the same rsocket_api folder and attached to the CSR
3. I updated the CSR content with finalized class names and function names
Please review and let me know if everything looks OK.
Again, thank you very much for your help!
Lucy
From: nio-dev [mailto:nio-dev-bounces at openjdk.java.net] On Behalf Of Lu, Yingqi
Sent: Friday, November 16, 2018 9:07 AM
To: Chris Hegarty <chris.hegarty at oracle.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
Hi Chris and Daniel,
Thank you very much for your timely help! I already generated the Javadoc. Will attach it to the CSR shortly.
Thanks,
Lucy
From: Chris Hegarty [mailto:chris.hegarty at oracle.com]
Sent: Friday, November 16, 2018 9:04 AM
To: Lu, Yingqi <yingqi.lu at intel.com<mailto:yingqi.lu at intel.com>>
Cc: nio-dev at openjdk.java.net<mailto:nio-dev at openjdk.java.net>; Viswanathan, Sandhya <sandhya.viswanathan at intel.com<mailto:sandhya.viswanathan at intel.com>>; Aundhe, Shirish <shirish.aundhe at intel.com<mailto:shirish.aundhe at intel.com>>; Kaczmarek, Eric <eric.kaczmarek at intel.com<mailto:eric.kaczmarek at intel.com>>
Subject: Re: adding rsockets support into JDK
Lucy,
On 16 Nov 2018, at 14:29, Lu, Yingqi <yingqi.lu at intel.com<mailto:yingqi.lu at intel.com>> wrote:
...
I already updated RdmaSockets.java in the version 17 of the patch with your
suggestions. http://cr.openjdk.java.net/~ylu/8195160.17/
The specification in jdk/net/RdmaSocketOptions.java and
jdk/net/RdmaSockets.java look good to me.
I have not generated Javadoc before. Would you please let me know what
kind of flags/options I need to use for the two classes?
Similar to what Daniel suggested. Here’s what I did with stubbed out
versions of the two above files from the `.17` webrev:
$ make docs
$ ...
$ mkdir -p rsocket_docs/api
$ cp build/macosx-x64/images/docs/api/stylesheet.css rsocket_docs/api/
$ cp build/macosx-x64/images/docs/api/index.html rsocket_docs/api/
$ cp -r build/macosx-x64/images/docs/api/jdk.net<http://jdk.net> rsocket_docs/api/
$ scp -r rsocket_docs/ chegar at cr.openjdk.java.net<mailto:chegar at cr.openjdk.java.net>:
Online docs:
http://cr.openjdk.java.net/~chegar/rsocket_docs/api/jdk.net/jdk/net/package-summary.html
Also, in addition to attach the Javadocs for the two classes, anything else I
need to modify the CSR?
At least an outline of the final classes should be included. No need
to duplicate the spec since you will be attaching a zipped bundle of the
docs. Good to include an online viewable link for convenience, and also
attach a zipped bundled of a minimal doc or specdiff for the archives.
-Chris.
Thanks,
Lucy
-----Original Message-----
From: Chris Hegarty [mailto:chris.hegarty at oracle.com]
Sent: Friday, November 16, 2018 3:19 AM
To: Lu, Yingqi <yingqi.lu at intel.com<mailto:yingqi.lu at intel.com>>
Cc: nio-dev at openjdk.java.net<mailto:nio-dev at openjdk.java.net>; Viswanathan, Sandhya
<sandhya.viswanathan at intel.com<mailto:sandhya.viswanathan at intel.com>>; Aundhe, Shirish
<shirish.aundhe at intel.com<mailto:shirish.aundhe at intel.com>>; Kaczmarek, Eric <eric.kaczmarek at intel.com<mailto:eric.kaczmarek at intel.com>>
Subject: Re: adding rsockets support into JDK
Lucy,
On 15/11/18 19:12, Lu, Yingqi wrote:
Hi Chris,
Here is the version 17 of the patch:
http://cr.openjdk.java.net/~ylu/8195160.17/
This looks much better. Some final spec comments:
1) src/jdk.net/share/classes/jdk/net/RdmaSocketOptions.java<http://jdk.net/share/classes/jdk/net/RdmaSocketOptions.java>
Looks good.
2) src/jdk.net/share/classes/jdk/net/RdmaSockets.java<http://jdk.net/share/classes/jdk/net/RdmaSockets.java>
a) class-level description now seems a little odd, and heavily weighted
towards channels, how about something like:
/**
* Factory methods for creating RDMA-based TCP sockets and channels.
*
* <p> The {@link #openSocket() openSocket} and {@link
* #openServerSocket() openServerSocket} methods create RDMA-based
* TCP sockets.
*
* <p> The {@link #openSelector() openSelector ...
b) Maybe put the security manager verbiage in its own paragraph, since
it relates to both sockets and channels.
* <p> When a security manager is installed ...
c) Please remove the trailing <p> on line L60.
d) openSocket: "Creates an unconnected RDMA socket."
Given that the socket options mention that the option is allowed to
be "set before the socket is bound or connected", then maybe this
opening sentence should be:
"Creates an unbound and unconnected RDMA socket.
Note: the other factory methods are clear on this point already.
3) Minor: personally I'd remove the @see ...NetworkChannel
It adds no value.
That's it.
Is it possible to generate the javadoc for these classes? It would be
useful to have to attach to the CSR, or even to be viewable as a link from the
CSR.
Can you please update the CSR and I will then add myself as reviewer?
-Chris.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20181116/b630b586/attachment-0001.html>
More information about the nio-dev
mailing list