Hi Chris,
On Wednesday 11 October 2017 12:28 AM, Chris Hegarty wrote:
Vyom,
What about suggestion 1) below, the name of the socket option?
to be consistent with SO_FLOW_SLA in ExtendedSocketOptions.java, i
choose the "SO" prefix. But I don't know the history behind the "SO"
prefix so i changed the socket option name to "TCP_QUICKACK" as suggested.
Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8145635/webrev0.3/index.html).
Thanks,
Vyom
-Chris.
On 27 Sep 2017, at 09:56, vyom tewari <vyom.tew...@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.tew...@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.
