pushed the
code(http://hg.openjdk.java.net/jdk10/jdk10/jdk/rev/7cdde79d6a46).
Vyom
On Friday 28 April 2017 03:26 AM, Langer, Christoph wrote:
Hi Vyom,
I’ve just got a small formatting remark about the order of includes:
Generally I tried to follow the rule 1. Common os headers, 2. Platform
os headers, 3. Jvm/jdk headers, 4. JNI headers in my latest
refactorings. So, to keep this up, can you move #include “jvm.h” in
the line before #include “net_util.h” in each file? And pull it before
the JNI headers. Like, e.g. in net_util_md.c you should move #include
"jvm.h" to line 52.
Thanks & Best regards
Christoph
*From:*net-dev [mailto:net-dev-boun...@openjdk.java.net] *On Behalf Of
*Vyom Tewari
*Sent:* Donnerstag, 27. April 2017 06:16
*To:* Thomas Stüfe <thomas.stu...@gmail.com>; Chris Hegarty
<chris.hega...@oracle.com>
*Cc:* net-dev <net-dev@openjdk.java.net>
*Subject:* Re: JDK10 RFR: 8165437 Evaluate the use of gettimeofday in
Networking code
Hi,
please find the updated
webrev(http://cr.openjdk.java.net/~vtewari/8165437/webrev0.7/index.html
<http://cr.openjdk.java.net/%7Evtewari/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.hega...@oracle.com <mailto:chris.hega...@oracle.com>> wrote:
> On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari
<vyom.tew...@oracle.com <mailto:vyom.tew...@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.stu...@gmail.com <mailto:thomas.stu...@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.
