JDK10 RFR: 8165437 Evaluate the use of gettimeofday in Networking code

[Vyom Tewari]

Hi,

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

Thanks,

Vyom



On Tuesday 25 April 2017 07:34 PM, Thomas Stüfe wrote:
> Hi Chris, Vyom,
>
> I have preferences as expressed earlier, but no strong emotions. I can 
> live with the fix as it is now.
>
> Thanks all, and Kind Regards, Thomas
>
>
> On Tue, Apr 25, 2017 at 3:31 PM, Chris Hegarty 
> <chris.hegarty at oracle.com <mailto:chris.hegarty at oracle.com>> wrote:
>
>     > On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari <vyom.tewari at oracle.com
>     <mailto:vyom.tewari at oracle.com>> wrote:
>     > ...
>     >
>     > Thanks for review, please find the updated
>     webrev(http://cr.openjdk.java.net/~vtewari/8165437/webrev0.6/index.html
>     <http://cr.openjdk.java.net/%7Evtewari/8165437/webrev0.6/index.html>)
>
>     The changes mainly look good to me, just a few comments:
>
>     1) src/java.base/unix/native/libnet/PlainSocketImpl.c
>
>        L235     jlong nanoTimeout = timeout * NET_NSEC_PER_MSEC;
>
>        Can you please move this to the latest block of code that
>     requires it, i.e..
>        just after L327 if (connect_rv != 0) { …
>
>     2) src/java.base/share/native/libnet/net_util.h
>
>        Should these definitions be moved to
>     src/java.base/unix/native/libnet/net_util_md.h?
>
>
>     Regarding JVM_NanoTime being a JVM_LEAF and/or taking a JNIEnv (
>     both of which are, in todays hotspot VM, effectively ignored ),
>     this is a
>     separate issue. I have raised it off list with others from the VM
>     team,
>     without much interest. I will refrain from filing a JIRA issue to
>     track potential
>     changes in the VM for this. Others are welcome to do so, if they
>     wish. I
>     suggest that we simply continue to pass a valid JNIEnv, since it
>     is not
>     much extra effort to do so. ( this can be refactored later, if the
>     VM interface
>     is ever updated ).
>
>
>     > On 24 Apr 2017, at 12:07, Thomas Stüfe <thomas.stuefe at gmail.com
>     <mailto:thomas.stuefe at gmail.com>> wrote:
>     >
>     > ...
>     > That aside, I am not a big fan of the removal of the old NET_Timeout. Before, there were two
>     functions, "NET_Timeout" just taking the timeout value,
>     "NET_Timeout0" taking a timeout and a start value. You removed the
>     first variant and therefore added the many additional
>     JVM_NanoTime() calls at NET_Timeout callsites. This makes the code
>     harder to read and also kind of exposes the internal
>     implementation of NET_Timeout (namely the fact that NET_Timeout
>     uses JVM_NanoTime for time measurement). Could you please let both
>     variants live, optionally with better names?
>
>     I think that it may not be worth added back the simpler variant. It
>     would only be used by PlainDatagramSocketImpl, correct? All other
>     usages would use the variant that accepts the current nano time.
>
>     -Chris.
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20170427/76d3325b/attachment.html>