RFR 8145635 : Add TCP_QUICKACK socket option

[vyom tewari]

Hi All,

please find the latest 
webrev(http://cr.openjdk.java.net/~vtewari/8145635/webrev0.7/index.html).

Thanks,

Vyom


On Wednesday 18 October 2017 08:21 PM, Langer, Christoph wrote:
> Hi Vyom,
>
> I just looked at your latest webrev which in general looks fine to me.  Some minor remarks:
>
> make/lib/Lib-jdk.net.gmk:
> - copyright year needs to be updated
>
> src/java.base/unix/classes/java/net/PlainDatagramSocketImpl.java:
> - private void addExtSocketOptions(...) looks a bit awful concerning its formatting. I suggest something like this:
>
> private void addExtSocketOptions(Set<SocketOption<?>> extOptions,
>                                   Set<SocketOption<?>> options) {
>      // TCP_QUICKACK is applicable for TCP Sockets only.
>      extOptions.stream()
>          .filter((option) -> !option.name().equals("TCP_QUICKACK"))
>          .forEach((option) -> options.add(option));
> }
>
> src/jdk.net/share/classes/jdk/net/ExtendedSocketOptions.java:
> - copyright year needs to be updated
> - the equals sign should be placed on line 98 here:
>
> 98     public static final SocketOption<Boolean> TCP_QUICKACK
> 99             = new ExtSocketOption<Boolean>("TCP_QUICKACK", Boolean.class);
>
> Other than that you should consider it reviewed from my end. No need for further webrev...
>
> Best regards
> Christoph
>
>> -----Original Message-----
>> From: net-dev [mailto:net-dev-bounces at openjdk.java.net] On Behalf Of
>> vyom tewari
>> Sent: Dienstag, 17. Oktober 2017 10:37
>> To: net-dev at openjdk.java.net
>> Subject: Re: RFR 8145635 : Add TCP_QUICKACK socket option
>>
>> Hi Roger,
>>
>> Thanks for the review , please find the updated
>> webrev(http://cr.openjdk.java.net/~vtewari/8145635/webrev0.6/index.htm
>> l).
>>
>> Thanks,
>>
>> Vyom
>>
>>
>> On Tuesday 17 October 2017 06:35 AM, Roger Riggs wrote:
>>> Hi Vyom,
>>>
>>> A few suggestions:
>>>
>>> PlainDatagramSocketImpl.java:
>>>   - line 95/96:  I think you can use just forEach, the order version is
>>> not necessary.
>>>      The code will be a bit more readable if the .filter and .forEach
>>> are on a new line and don't wrap.
>>>      You can also remove the extra "(" and ")
>>>
>>>   - line 87-94: these are confusing and imply some implicit resetting
>>> of the option.
>>>   - use @since 10
>>> - 209/268: the native setQuickAck method should use boolean as its
>>> argument to enable/disable
>>>    Since enable is a boolean; it does not need "== true'
>>>
>>> LinuxSocketOptions.java/c:
>>>    - 52: setQuickAck0 should use boolean for the 2nd argument; (The
>>> native code already does)
>>>
>>> Thanks, Roger
>>>
>>>
>>> On 10/15/17 11:58 PM, vyom tewari wrote:
>>>> Hi Chris,
>>>>
>>>> Thanks for review. Please find the latest
>>>>
>> webrev(http://cr.openjdk.java.net/~vtewari/8145635/webrev0.5/index.htm
>> l).
>>>> Thanks,
>>>>
>>>> Vyom
>>>>