RFR:8194298 Add support for per Socket configuration of TCP keepalive

[Ivan Gerasimov]

Hi Vyom!

1)
src/java.base/share/classes/sun/net/ext/ExtendedSocketOptions.java

Thank you for fixing ExtendingSocketOption.options0().
It may be better to make the returned set unmodifiable, and then 
Collectors.toUnmodifiableSet could be used for convenience:

return options.stream()
         .filter((option) -> !option.name().startsWith("TCP_"))
         .collect(Collectors.toUnmodifiableSet());

Also, I think Alan suggested to filter out UDP_* options for SOCK_STREAM 
here.

2)
src/java.base/unix/classes/java/net/PlainDatagramSocketImpl.java

If you static-import ExtendedSocketOptions.SOCK_DGRAM as in other files, 
then the line 80 wouldn't become too long.

The same applies to src/java.base/unix/classes/java/net/PlainSocketImpl.java

3)
src/jdk.net/share/classes/jdk/net/ExtendedSocketOptions.java

Was it intentional that when flowSupported == true, quickAckSupported == 
false, keepAliveOptSupported == true, the TCP_KEEP* options are *not* 
added to the resulting set?

If not, then can this set population be organized in more linear way:  
Just create an ArrayList, conditionally fill it in and return 
unmodifiable set with Set.of(list.toArray()).

Nit:  please place the equal sign at line 172 consistently with the 
other two inits above.

With kind regards,
Ivan


On 5/11/18 7:43 PM, vyom tewari wrote:
> thanks all for review, please find the latest 
> webrev(http://cr.openjdk.java.net/~vtewari/8194298/webrev0.3/index.html). 
> I address most of the  review comments.
>
> Vyom
>
>
> On Saturday 12 May 2018 12:01 AM, Chris Hegarty wrote:
>> On 11 May 2018, at 01:04, Alan Bateman <Alan.Bateman at oracle.com> wrote:
>>> On 10/05/2018 16:21, vyom tewari wrote:
>>>> Hi,
>>>>
>>>> Please find the latest 
>>>> webrev(http://cr.openjdk.java.net/~vtewari/8194298/webrev0.2/index.html) 
>>>>
>>>> ...
>>> It would be better if the channel implementation didn't static 
>>> import ExtendedSocketOptions.getInstance as that is a very generic 
>>> method method name. As I mentioned previously, you could simplify 
>>> all these usages if you add the following to 
>>> sun.net.ext.ExtendedSocketOption
>>>     static Set<SocketOption<?>> options(int type) { return 
>>> getInstance().options(type)); }
>> +1
>>
>>> A minor comment on tests is that they can use List.of instead of 
>>> Arrays.asList.
>> +1
>>
>> Otherwise, this looks very good.
>>
>> -Chris.
>>
>> P.S. A separate issue, but when reviewing this it reminded me that we 
>> should deprecate-for-removal jdk/net/Sockets.java. It’s functionality 
>> is already supported by a standard API.
>>
>
>

-- 
With kind regards,
Ivan Gerasimov