RFR 8145635 : Add TCP_QUICKACK socket option
Chris Hegarty
chris.hegarty at oracle.com
Tue Oct 10 18:58:28 UTC 2017
Vyom,
What about suggestion 1) below, the name of the socket option?
-Chris.
> On 27 Sep 2017, at 09:56, vyom tewari <vyom.tewari at oracle.com> wrote:
>
> Hi Chris,
>
> Thanks for review, please find the latest webrev(http://cr.openjdk.java.net/~vtewari/8145635/webrev0.2/index.html). I incorporated review comments from you and re-base the patch to our consolidated repo(jdk10/master).
>
> Thanks,
>
> Vyom
>
>
> On Monday 25 September 2017 01:57 AM, Chris Hegarty wrote:
>> Vyom,
>>
>>> On 11 Sep 2017, at 16:38, vyom tewari <vyom.tewari at oracle.com> wrote:
>>>
>>> Hi All,
>>>
>>> As jdk.net API already moved out of java.base, Please review the below code change for jdk10.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8145635
>>> Webrev: http://cr.openjdk.java.net/~vtewari/8145635/webrev0.1/index.html
>>>
>> This looks quite good. Some comments:
>>
>> 1) I wonder if we should just call the option TCP_QUICKACK, rather than SO_QUICKACK? Similar to TCP_NODELAY.
>>
>> 2) I think LinuxSocketOptions.h is largely unnecessary, if we do 1) above.
>>
>> 3) Java_jdk_net_LinuxSocketOptions_getQuickAck could return jint, rather than jobject, thus avoiding the need for createBoolean. The conversation can happen in the Java layer. Can you please reduce the lint length in this file, by wrapping similar to the style of the Solaris version.
>>
>> 4) ExtendedSocketOptions.java
>> - drop the <p>, they are unnecessary.
>> - I think, similar to TCP_NODELAY, the spec should say that the options is TCP specific. From TCP_NODELAY: "The socket option is specific to stream-oriented sockets using the TCP/IP protocol.”
>> - "In TCP_QUICKACK mode”, maybe “When the option is enabled…”
>>
>> -Chris.
>>
>>
>
More information about the core-libs-dev
mailing list