Yes, since the functionality is basically duplicated across the two
mechanisms
and since setOption/getOption already exists, the only new part of the API
is the SO_REUSEPORT option itself in StandardSocketOptions.
- Michael
On 07/12/15 17:04, Lu, Yingqi wrote:
Hi Alan and Michael,
Thank you very much for your detailed feedback. Before I start modify
the code, I want to make sure I understand correctly. Regarding the
following comment, do you mean that we should remove the setReusePort
and getReusePort methods? If yes, we can easily remove them, and keep
all the setOption/getOption methods the same way for SO_REUSEPORT.
I agree with Michael, we should decide whether it make sense to add
specific methods to Socket/ServerSocket/DatagramSocket. They aren't
strictly needed and given that the socket option is essentially
optional then setOption/getOption should be fine.
I will work on this today. Hopefully can send out an update tomorrow.
Thanks,
Lucy
*From:*Alan Bateman [mailto:alan.bate...@oracle.com]
*Sent:* Monday, December 07, 2015 7:04 AM
*To:* Lu, Yingqi <yingqi...@intel.com>
*Cc:* net-dev@openjdk.java.net; Kharbas, Kishor
<kishor.khar...@intel.com>; Kaczmarek, Eric <eric.kaczma...@intel.com>
*Subject:* Re: Patch for adding SO_REUSEPORT socket option
On 02/12/2015 17:08, Lu, Yingqi wrote:
Hi All,
A quick check here. Does anyone get a chance to try the most
recent patch? Any feedback and comments?
Thanks,
Lucy
I looked through the latest webrev (webrev.05) and it's looking quite
good.
I agree with Michael, we should decide whether it make sense to add
specific methods to Socket/ServerSocket/DatagramSocket. They aren't
strictly needed and given that the socket option is essentially
optional then setOption/getOption should be fine.
The updated wording in StandardSocketOptions looks okay but one
suggestion is to include the word "usually" as it can't mandate the
behavior on all platforms where it is supported - for example "the
socket option will usually allow ...". A minor point here is that they
needs @since 9.
The updates to the NIO implementation classes mostly look okay, except
sun.nio.ch.Net where I have a number of comments:
1. In Net.java then you'll see where it caches several capabilities,
that could be used to cache whether this socket option is supported or
not, no need to do it in native code (Net.c).
2. I see reuseportSupported() is public so that it can be in several
places (including code in java.net) but would be nice to avoid that.
3. For #1 and #3 then maybe the simplest is to add a native function
in libnet like we have for IPv6 and that would allow the net and nio
code to use the same implementation that is consistent with the
existing code.
4. Minor comment on Net.c is that you use UNUSED whereas the existing
code uses "this".
I wonder if it might be simpler to leave the debugger agent out of
this, I don't see a big reason to change it.
For the tests then it would be good to avoid using
sun.net.ch.Net.reuseportSupported(). Instead I think it should check
that supportedOptions returns the right value for the platform.
I assume tests such as MulticastSendReceiveTests don't need to set
this option.
-Alan.
