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

[David Holmes]

On 4/05/2017 2:07 PM, Vyom Tewari wrote:

Hi David,

I will look into the issue.


Thanks. I filed:

https://bugs.openjdk.java.net/browse/JDK-8179602

as you probably saw.

David



Thanks,

Vyom


On Thursday 04 May 2017 06:29 AM, David Holmes wrote:

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



This change is broken on 32-bit systems - JVM_Nanotime returns a jlong
which is always 64-bit, but the code uses long for the nanotimeout
values, which will be 32-bit on 32-bit systems!

This change appears to be causing massive testing failures due to
jtreg agentvm failing due to sock connection issues. Most obvious with
32-bit linux builds. However we also see a some failures on OSX which
is not explained by the long vs jlong issue.

David
-

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

[Vyom Tewari]

Hi David,

I will look into the issue.

Thanks,

Vyom


On Thursday 04 May 2017 06:29 AM, David Holmes wrote:

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



This change is broken on 32-bit systems - JVM_Nanotime returns a jlong 
which is always 64-bit, but the code uses long for the nanotimeout 
values, which will be 32-bit on 32-bit systems!


This change appears to be causing massive testing failures due to 
jtreg agentvm failing due to sock connection issues. Most obvious with 
32-bit linux builds. However we also see a some failures on OSX which 
is not explained by the long vs jlong issue.


David
-

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

[David Holmes]

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


This change is broken on 32-bit systems - JVM_Nanotime returns a jlong 
which is always 64-bit, but the code uses long for the nanotimeout 
values, which will be 32-bit on 32-bit systems!


This change appears to be causing massive testing failures due to jtreg 
agentvm failing due to sock connection issues. Most obvious with 32-bit 
linux builds. However we also see a some failures on OSX which is not 
explained by the long vs jlong issue.


David
-

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

[Vyom Tewari]

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 ; Chris Hegarty 


*Cc:* net-dev 
*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 
).


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
> wrote:

> On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari
> wrote:
> ...
>
> Thanks for review, please find the updated
webrev(http://cr.openjdk.java.net/~vtewari/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
> 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.

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

[Langer, Christoph]

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 ; Chris Hegarty 

Cc: net-dev 
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).

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 
> wrote:
> On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari 
> > wrote:
> ...
>
> Thanks for review, please find the updated 
> webrev(http://cr.openjdk.java.net/~vtewari/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 
> > 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.

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

[Vyom Tewari]

Hi,

Even i thought the same, pass nanosecond timeout to "NET_Timeout" but if 
you see the implementation it uses " *int poll(struct pollfd **/fds/*, 
nfds_t */nfds/*, int */timeout/*);*


"  where timeout is in millisecond so we have to do conversion anyway in 
loop if we pass timeout in NS. So there will not be much difference,  
will save just one conversion.


thanks,

Vyom

On Thursday 27 April 2017 04:58 PM, Bernd Eckenfels wrote:

Hello,

It looks to me like using nanoseconds in the NET_Timeout Timeout 
Parameter would remove quite a few conversions. Callsides mostly 
already have timeoutNanoseconds for calculating reminder.


Gruss
Bernd
--
http://bernd.eckenfels.net


*From:* net-dev > on behalf of Thomas Stüfe 
>

*Sent:* Monday, April 24, 2017 1:07:52 PM
*To:* Vyom Tewari
*Cc:* net-dev
*Subject:* Re: JDK10 RFR: 8165437 Evaluate the use of gettimeofday in 
Networking code

Hi Vyom,

sorry for the late response, I had vacation.

Thanks for taking my suggestions! Here some remarks:

---

I looked a little bit closer into the question why JVM_LEAF is used to 
wrap simple little functions like JVM_NanoTime or 
JVM_CurrentTimeMillis (among others). There is no hard technical 
reason why a function could not just be exported from the libjvm.so 
using JNIEXPORT, in any form one wishes too. (In fact, we do this in 
our jvm port a lot).


However, JVM_NanoTime() and JVM_CurrentTimeMillis() are used as direct 
native implementations for System.currentTimeMillis() and 
System.nanoTime() (see share/native/libjava/System.c), using 
RegisterNatives(). So, the original author saved the glue functions he 
would have to write otherwise (Java_java_lang_System_currentTimeMillis 
etc).


The comments in System.c indicate that this was done for performance 
reasons (you save one call frame by calling JVM_NanoTime directly).


Because they are used as direct JNI implementations for static java 
methods in System, JVM_NanoTime() and JVM_CurrentTimeMillis() have to 
carry around JNIEnv* and jclass parameters. But they are ignored by 
the functions - the jclass argument is even called "ignored" in jvm.h. 
And it seems to be perfectly okay to call JVM_CurrentTimeMillis() with 
NULL as JNIEnv* argument, which is done in many places in the jdk. 
Presumably this would be okay too for JVM_NanoTime(), so you could at 
least avoid the new JNIEnv* argument in NET_Timeout and just call 
JVM_NanoTime with NULL as first parameter.


-

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?


-

Otherwise, I think the change looks good. Thanks for your patience!

Kind Regards, Thomas





On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari > wrote:


Hi Thomas,

Thanks for review, please find the updated
webrev(http://cr.openjdk.java.net/~vtewari/8165437/webrev0.6/index.html
)
i incorporated all the review comments. Regarding why JVM_NanoTime
is defined JVM_LEAF i don't know much, but all the functions which
are defined in jvm.h used some sort of
macro(JVM_LEAF,JVM_ENTRY,JVM_ENTRY_NO_ENV etc). You can not call
"os::javaTimeNanos()" directly as this is not visible, i did a
small prototype which simply export this without any JVM macro as
below.

This prototype did worked but i am not sure if this is right way
to do. I need some more input, why all jvm.h functions are defined
with macro and if it OK to defined function as below, maybe some
one from JVM team can give some more comment on this. I decided
not to include this prototype as part of review because i am not
sure if this is right way.

Thanks,

Vyom

##

diff -r 26d689c621f6 make/symbols/symbols-unix
--- a/make/symbols/symbols-unixWed Apr 12 19:28:47 2017 -0700
+++ b/make/symbols/symbols-unixTue Apr 18 08:40:25 2017 +0530
@@ -51,6 +51,7 @@
 JVM_CurrentLoadedClass
 JVM_CurrentThread
 JVM_CurrentTimeMillis
+JVM_CurrentTimeNano
 JVM_DefineClass
 JVM_DefineClassWithSource
 JVM_DesiredAssertionStatus
diff -r 26d689c621f6 src/share/vm/prims/jvm.cpp
--- 

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

[Bernd Eckenfels]

Hello,

It looks to me like using nanoseconds in the NET_Timeout Timeout Parameter 
would remove quite a few conversions. Callsides mostly already have 
timeoutNanoseconds for calculating reminder.

Gruss
Bernd
--
http://bernd.eckenfels.net


From: net-dev 
> on 
behalf of Thomas Stüfe >
Sent: Monday, April 24, 2017 1:07:52 PM
To: Vyom Tewari
Cc: net-dev
Subject: Re: JDK10 RFR: 8165437 Evaluate the use of gettimeofday in Networking 
code

Hi Vyom,

sorry for the late response, I had vacation.

Thanks for taking my suggestions! Here some remarks:

---

I looked a little bit closer into the question why JVM_LEAF is used to wrap 
simple little functions like JVM_NanoTime or JVM_CurrentTimeMillis (among 
others). There is no hard technical reason why a function could not just be 
exported from the libjvm.so using JNIEXPORT, in any form one wishes too. (In 
fact, we do this in our jvm port a lot).

However, JVM_NanoTime() and JVM_CurrentTimeMillis() are used as direct native 
implementations for System.currentTimeMillis() and System.nanoTime() (see 
share/native/libjava/System.c), using RegisterNatives(). So, the original 
author saved the glue functions he would have to write otherwise 
(Java_java_lang_System_currentTimeMillis etc).

The comments in System.c indicate that this was done for performance reasons 
(you save one call frame by calling JVM_NanoTime directly).

Because they are used as direct JNI implementations for static java methods in 
System, JVM_NanoTime() and JVM_CurrentTimeMillis() have to carry around JNIEnv* 
and jclass parameters. But they are ignored by the functions - the jclass 
argument is even called "ignored" in jvm.h. And it seems to be perfectly okay 
to call JVM_CurrentTimeMillis() with NULL as JNIEnv* argument, which is done in 
many places in the jdk. Presumably this would be okay too for JVM_NanoTime(), 
so you could at least avoid the new JNIEnv* argument in NET_Timeout and just 
call JVM_NanoTime with NULL as first parameter.

-

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?

-

Otherwise, I think the change looks good. Thanks for your patience!

Kind Regards, Thomas






On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari 
> wrote:

Hi Thomas,

Thanks for review, please find the updated 
webrev(http://cr.openjdk.java.net/~vtewari/8165437/webrev0.6/index.html) i 
incorporated all the review comments. Regarding why JVM_NanoTime is defined 
JVM_LEAF i don't know much, but all the functions which are defined in jvm.h 
used some sort of macro(JVM_LEAF,JVM_ENTRY,JVM_ENTRY_NO_ENV etc). You can not 
call "os::javaTimeNanos()" directly as this is not visible, i did a small 
prototype which simply export this without any JVM macro as below.

This prototype did worked but i am not sure if this is right way to do. I need 
some more input, why all jvm.h functions are defined with macro and if it OK to 
defined function as below, maybe some one from JVM team can give some more 
comment on this. I decided not to include this prototype as part of review 
because i am not  sure if this is right way.

Thanks,

Vyom

##

diff -r 26d689c621f6 make/symbols/symbols-unix
--- a/make/symbols/symbols-unixWed Apr 12 19:28:47 2017 -0700
+++ b/make/symbols/symbols-unixTue Apr 18 08:40:25 2017 +0530
@@ -51,6 +51,7 @@
 JVM_CurrentLoadedClass
 JVM_CurrentThread
 JVM_CurrentTimeMillis
+JVM_CurrentTimeNano
 JVM_DefineClass
 JVM_DefineClassWithSource
 JVM_DesiredAssertionStatus
diff -r 26d689c621f6 src/share/vm/prims/jvm.cpp
--- a/src/share/vm/prims/jvm.cppWed Apr 12 19:28:47 2017 -0700
+++ b/src/share/vm/prims/jvm.cppTue Apr 18 08:40:25 2017 +0530
@@ -273,7 +273,11 @@
   JVMWrapper("JVM_NanoTime");
   return os::javaTimeNanos();
 JVM_END
-
+
+long JVM_CurrentTimeNano(){
+return os::javaTimeNanos();
+}
+
 // The function below is actually exposed by jdk.internal.misc.VM and not
 // java.lang.System, but we choose to keep it here so that it stays next
 // to JVM_CurrentTimeMillis and JVM_NanoTime
diff -r 26d689c621f6 src/share/vm/prims/jvm.h
--- a/src/share/vm/prims/jvm.hWed Apr 12 19:28:47 2017 -0700
+++ b/src/share/vm/prims/jvm.hTue Apr 18 08:40:25 2017 +0530
@@ -119,6 +119,9 @@
 JNIEXPORT jlong JNICALL
 

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

[Chris Hegarty]

> On 27 Apr 2017, at 05:15, Vyom Tewari  wrote:
> 
> Hi,
> 
> please find the updated 
> webrev(http://cr.openjdk.java.net/~vtewari/8165437/webrev0.7/index.html).

This looks ok to me Vyom, but I think you have misinterpreted my comment...

>> ...
>> 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) { …

You seem to have moved this line too far, the declaration should
be at the beginning of the if block, just before the  prevNanoTime
declaration and assignment.  No need for another webrev, you
can just change it before pushing.

-Chris.

Re: 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 
> wrote:


> On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari > wrote:
> ...
>
> Thanks for review, please find the updated
webrev(http://cr.openjdk.java.net/~vtewari/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 > 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.

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

[Thomas Stüfe]

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 
wrote:

> > On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari 
> wrote:
> > ...
> >
> > Thanks for review, please find the updated webrev(
> http://cr.openjdk.java.net/~vtewari/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  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.
>
>

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

[Chris Hegarty]

> On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari  wrote:
> ...
> 
> Thanks for review, please find the updated 
> webrev(http://cr.openjdk.java.net/~vtewari/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  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.

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

[Bernd Eckenfels]

Can't you just keep a NET_Timeout using directly os::javaTimeNano()?

Gruss
Bernd
--
http://bernd.eckenfels.net

From: net-dev  on behalf of Thomas Stüfe 

Sent: Monday, April 24, 2017 1:07:52 PM
To: Vyom Tewari
Cc: net-dev
Subject: Re: JDK10 RFR: 8165437 Evaluate the use of gettimeofday in Networking 
code

Hi Vyom,

sorry for the late response, I had vacation.

Thanks for taking my suggestions! Here some remarks:

---

I looked a little bit closer into the question why JVM_LEAF is used to wrap 
simple little functions like JVM_NanoTime or JVM_CurrentTimeMillis (among 
others). There is no hard technical reason why a function could not just be 
exported from the libjvm.so using JNIEXPORT, in any form one wishes too. (In 
fact, we do this in our jvm port a lot).

However, JVM_NanoTime() and JVM_CurrentTimeMillis() are used as direct native 
implementations for System.currentTimeMillis() and System.nanoTime() (see 
share/native/libjava/System.c), using RegisterNatives(). So, the original 
author saved the glue functions he would have to write otherwise 
(Java_java_lang_System_currentTimeMillis etc).

The comments in System.c indicate that this was done for performance reasons 
(you save one call frame by calling JVM_NanoTime directly).

Because they are used as direct JNI implementations for static java methods in 
System, JVM_NanoTime() and JVM_CurrentTimeMillis() have to carry around JNIEnv* 
and jclass parameters. But they are ignored by the functions - the jclass 
argument is even called "ignored" in jvm.h. And it seems to be perfectly okay 
to call JVM_CurrentTimeMillis() with NULL as JNIEnv* argument, which is done in 
many places in the jdk. Presumably this would be okay too for JVM_NanoTime(), 
so you could at least avoid the new JNIEnv* argument in NET_Timeout and just 
call JVM_NanoTime with NULL as first parameter.

-

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?

-

Otherwise, I think the change looks good. Thanks for your patience!

Kind Regards, Thomas






On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari 
> wrote:

Hi Thomas,

Thanks for review, please find the updated 
webrev(http://cr.openjdk.java.net/~vtewari/8165437/webrev0.6/index.html) i 
incorporated all the review comments. Regarding why JVM_NanoTime is defined 
JVM_LEAF i don't know much, but all the functions which are defined in jvm.h 
used some sort of macro(JVM_LEAF,JVM_ENTRY,JVM_ENTRY_NO_ENV etc). You can not 
call "os::javaTimeNanos()" directly as this is not visible, i did a small 
prototype which simply export this without any JVM macro as below.

This prototype did worked but i am not sure if this is right way to do. I need 
some more input, why all jvm.h functions are defined with macro and if it OK to 
defined function as below, maybe some one from JVM team can give some more 
comment on this. I decided not to include this prototype as part of review 
because i am not  sure if this is right way.

Thanks,

Vyom

##

diff -r 26d689c621f6 make/symbols/symbols-unix
--- a/make/symbols/symbols-unixWed Apr 12 19:28:47 2017 -0700
+++ b/make/symbols/symbols-unixTue Apr 18 08:40:25 2017 +0530
@@ -51,6 +51,7 @@
 JVM_CurrentLoadedClass
 JVM_CurrentThread
 JVM_CurrentTimeMillis
+JVM_CurrentTimeNano
 JVM_DefineClass
 JVM_DefineClassWithSource
 JVM_DesiredAssertionStatus
diff -r 26d689c621f6 src/share/vm/prims/jvm.cpp
--- a/src/share/vm/prims/jvm.cppWed Apr 12 19:28:47 2017 -0700
+++ b/src/share/vm/prims/jvm.cppTue Apr 18 08:40:25 2017 +0530
@@ -273,7 +273,11 @@
   JVMWrapper("JVM_NanoTime");
   return os::javaTimeNanos();
 JVM_END
-
+
+long JVM_CurrentTimeNano(){
+return os::javaTimeNanos();
+}
+
 // The function below is actually exposed by jdk.internal.misc.VM and not
 // java.lang.System, but we choose to keep it here so that it stays next
 // to JVM_CurrentTimeMillis and JVM_NanoTime
diff -r 26d689c621f6 src/share/vm/prims/jvm.h
--- a/src/share/vm/prims/jvm.hWed Apr 12 19:28:47 2017 -0700
+++ b/src/share/vm/prims/jvm.hTue Apr 18 08:40:25 2017 +0530
@@ -119,6 +119,9 @@
 JNIEXPORT jlong JNICALL
 JVM_NanoTime(JNIEnv *env, jclass ignored);

+JNIEXPORT jlong
+JVM_CurrentTimeNano();
+
 JNIEXPORT jlong JNICALL
 JVM_GetNanoTimeAdjustment(JNIEnv *env, jclass ignored, jlong offset_secs);

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

[Thomas Stüfe]

Hi Vyom,

sorry for the late response, I had vacation.

Thanks for taking my suggestions! Here some remarks:

---

I looked a little bit closer into the question why JVM_LEAF is used to wrap
simple little functions like JVM_NanoTime or JVM_CurrentTimeMillis (among
others). There is no hard technical reason why a function could not just be
exported from the libjvm.so using JNIEXPORT, in any form one wishes too.
(In fact, we do this in our jvm port a lot).

However, JVM_NanoTime() and JVM_CurrentTimeMillis() are used as direct
native implementations for System.currentTimeMillis() and System.nanoTime()
(see share/native/libjava/System.c), using RegisterNatives(). So, the
original author saved the glue functions he would have to write otherwise
(Java_java_lang_System_currentTimeMillis etc).

The comments in System.c indicate that this was done for performance
reasons (you save one call frame by calling JVM_NanoTime directly).

Because they are used as direct JNI implementations for static java methods
in System, JVM_NanoTime() and JVM_CurrentTimeMillis() have to carry around
JNIEnv* and jclass parameters. But they are ignored by the functions - the
jclass argument is even called "ignored" in jvm.h. And it seems to be
perfectly okay to call JVM_CurrentTimeMillis() with NULL as JNIEnv*
argument, which is done in many places in the jdk. Presumably this would be
okay too for JVM_NanoTime(), so you could at least avoid the new JNIEnv*
argument in NET_Timeout and just call JVM_NanoTime with NULL as first
parameter.

-

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?

-

Otherwise, I think the change looks good. Thanks for your patience!

Kind Regards, Thomas






On Tue, Apr 18, 2017 at 7:08 AM, Vyom Tewari  wrote:

> Hi Thomas,
>
> Thanks for review, please find the updated webrev(http://cr.openjdk.java.
> net/~vtewari/8165437/webrev0.6/index.html) i incorporated all the review
> comments. Regarding why JVM_NanoTime is defined JVM_LEAF i don't know much,
> but all the functions which are defined in jvm.h used some sort of
> macro(JVM_LEAF,JVM_ENTRY,JVM_ENTRY_NO_ENV etc). You can not call
> "os::javaTimeNanos()" directly as this is not visible, i did a small
> prototype which simply export this without any JVM macro as below.
>
> This prototype did worked but i am not sure if this is right way to do. I
> need some more input, why all jvm.h functions are defined with macro and if
> it OK to defined function as below, maybe some one from JVM team can give
> some more comment on this. I decided not to include this prototype as part
> of review because i am not  sure if this is right way.
>
> Thanks,
>
> Vyom
>
> ##
>
> diff -r 26d689c621f6 make/symbols/symbols-unix
> --- a/make/symbols/symbols-unixWed Apr 12 19:28:47 2017 -0700
> +++ b/make/symbols/symbols-unixTue Apr 18 08:40:25 2017 +0530
> @@ -51,6 +51,7 @@
>  JVM_CurrentLoadedClass
>  JVM_CurrentThread
>  JVM_CurrentTimeMillis
> +JVM_CurrentTimeNano
>  JVM_DefineClass
>  JVM_DefineClassWithSource
>  JVM_DesiredAssertionStatus
> diff -r 26d689c621f6 src/share/vm/prims/jvm.cpp
> --- a/src/share/vm/prims/jvm.cppWed Apr 12 19:28:47 2017 -0700
> +++ b/src/share/vm/prims/jvm.cppTue Apr 18 08:40:25 2017 +0530
> @@ -273,7 +273,11 @@
>JVMWrapper("JVM_NanoTime");
>return os::javaTimeNanos();
>  JVM_END
> -
> +
> +long JVM_CurrentTimeNano(){
> +return os::javaTimeNanos();
> +}
> +
>  // The function below is actually exposed by jdk.internal.misc.VM and not
>  // java.lang.System, but we choose to keep it here so that it stays next
>  // to JVM_CurrentTimeMillis and JVM_NanoTime
> diff -r 26d689c621f6 src/share/vm/prims/jvm.h
> --- a/src/share/vm/prims/jvm.hWed Apr 12 19:28:47 2017 -0700
> +++ b/src/share/vm/prims/jvm.hTue Apr 18 08:40:25 2017 +0530
> @@ -119,6 +119,9 @@
>  JNIEXPORT jlong JNICALL
>  JVM_NanoTime(JNIEnv *env, jclass ignored);
>
> +JNIEXPORT jlong
> +JVM_CurrentTimeNano();
> +
>  JNIEXPORT jlong JNICALL
>  JVM_GetNanoTimeAdjustment(JNIEnv *env, jclass ignored, jlong
> offset_secs);
> ##
>
> On Thursday 13 April 2017 12:40 PM, Thomas Stüfe wrote:
>
> Hi Vyom,
>
> Thank you for fixing this!
>
> In addition to Rogers remarks:
>
> aix_close.c:
> Could you please also update the SAP copyright?
>
> style nit:
> +//nanoTimeout has to be >= 1 millisecond to iterate again.
> I thought we use 

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

[Vyom Tewari]

Hi Thomas,

Thanks for review, please find the updated 
webrev(http://cr.openjdk.java.net/~vtewari/8165437/webrev0.6/index.html) 
i incorporated all the review comments. Regarding why JVM_NanoTime is 
defined JVM_LEAF i don't know much, but all the functions which are 
defined in jvm.h used some sort of 
macro(JVM_LEAF,JVM_ENTRY,JVM_ENTRY_NO_ENV etc). You can not call 
"os::javaTimeNanos()" directly as this is not visible, i did a small 
prototype which simply export this without any JVM macro as below.


This prototype did worked but i am not sure if this is right way to do. 
I need some more input, why all jvm.h functions are defined with macro 
and if it OK to defined function as below, maybe some one from JVM team 
can give some more comment on this. I decided not to include this 
prototype as part of review because i am not sure if this is right way.


Thanks,

Vyom

##

diff -r 26d689c621f6 make/symbols/symbols-unix
--- a/make/symbols/symbols-unixWed Apr 12 19:28:47 2017 -0700
+++ b/make/symbols/symbols-unixTue Apr 18 08:40:25 2017 +0530
@@ -51,6 +51,7 @@
 JVM_CurrentLoadedClass
 JVM_CurrentThread
 JVM_CurrentTimeMillis
+JVM_CurrentTimeNano
 JVM_DefineClass
 JVM_DefineClassWithSource
 JVM_DesiredAssertionStatus
diff -r 26d689c621f6 src/share/vm/prims/jvm.cpp
--- a/src/share/vm/prims/jvm.cppWed Apr 12 19:28:47 2017 -0700
+++ b/src/share/vm/prims/jvm.cppTue Apr 18 08:40:25 2017 +0530
@@ -273,7 +273,11 @@
   JVMWrapper("JVM_NanoTime");
   return os::javaTimeNanos();
 JVM_END
-
+
+long JVM_CurrentTimeNano(){
+return os::javaTimeNanos();
+}
+
 // The function below is actually exposed by jdk.internal.misc.VM and not
 // java.lang.System, but we choose to keep it here so that it stays next
 // to JVM_CurrentTimeMillis and JVM_NanoTime
diff -r 26d689c621f6 src/share/vm/prims/jvm.h
--- a/src/share/vm/prims/jvm.hWed Apr 12 19:28:47 2017 -0700
+++ b/src/share/vm/prims/jvm.hTue Apr 18 08:40:25 2017 +0530
@@ -119,6 +119,9 @@
 JNIEXPORT jlong JNICALL
 JVM_NanoTime(JNIEnv *env, jclass ignored);

+JNIEXPORT jlong
+JVM_CurrentTimeNano();
+
 JNIEXPORT jlong JNICALL
 JVM_GetNanoTimeAdjustment(JNIEnv *env, jclass ignored, jlong offset_secs);
##


On Thursday 13 April 2017 12:40 PM, Thomas Stüfe wrote:

Hi Vyom,

Thank you for fixing this!

In addition to Rogers remarks:

aix_close.c:
Could you please also update the SAP copyright?

style nit:
+//nanoTimeout has to be >= 1 millisecond to iterate again.
I thought we use old C style comments? Could you please leave a space 
between comment and text?


net_util.h: It could be more readable if you used the "1000 * 1000..." 
notation to define the constants.


--

Apart from this, I have some small reservations about JVM_NanoTime: I 
see that JVM_NanoTime is defined using the JVM_LEAF macro. I am not 
sure why this is necessary. It has a number of disadvantages:


- we need to hand in JVMEnv*, which is not necessary for the time 
measurement itself (which is a simple os::javaTimeNanos() call). This 
requires us to funnel the JVMEnv* thru to NET_Timeout, which makes the 
caller code more complicated.


- JVM_LEAF does a number of verifications in the debug VM, which is 
not ideal for a time measure function


- Also, it blocks on VM exit. Probably not a problem, but a cause for 
potential problems.


os::javaTimeNanos is a simple time measuring function which does not 
depend on any internal VM state, as far as I see... so, I do not think 
it is necessary to wrap it with JVM_LEAF, no?


Kind Regards, Thomas


On Tue, Apr 11, 2017 at 3:04 PM, Vyom Tewari > wrote:


Hi,

Please review the code change for below issue.

Bug: https://bugs.openjdk.java.net/browse/JDK-8165437


Webrev:
http://cr.openjdk.java.net/~vtewari/8165437/webrev0.5/index.html


This change will replace the "gettimeofday" to use the monotonic
increasing clock(i used the existing function "JVM_NanoTime"
instead of writing my own).

Thanks,

Vyom

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

[Thomas Stüfe]

Hi Vyom,

Thank you for fixing this!

In addition to Rogers remarks:

aix_close.c:
Could you please also update the SAP copyright?

style nit:
+//nanoTimeout has to be >= 1 millisecond to iterate again.
I thought we use old C style comments? Could you please leave a space
between comment and text?

net_util.h: It could be more readable if you used the "1000 * 1000..."
notation to define the constants.

--

Apart from this, I have some small reservations about JVM_NanoTime: I see
that JVM_NanoTime is defined using the JVM_LEAF macro. I am not sure why
this is necessary. It has a number of disadvantages:

- we need to hand in JVMEnv*, which is not necessary for the time
measurement itself (which is a simple os::javaTimeNanos() call). This
requires us to funnel the JVMEnv* thru to NET_Timeout, which makes the
caller code more complicated.

- JVM_LEAF does a number of verifications in the debug VM, which is not
ideal for a time measure function

- Also, it blocks on VM exit. Probably not a problem, but a cause for
potential problems.

os::javaTimeNanos is a simple time measuring function which does not depend
on any internal VM state, as far as I see... so, I do not think it is
necessary to wrap it with JVM_LEAF, no?

Kind Regards, Thomas


On Tue, Apr 11, 2017 at 3:04 PM, Vyom Tewari  wrote:

> Hi,
>
> Please review the code change for below issue.
>
> Bug:   https://bugs.openjdk.java.net/browse/JDK-8165437
>
> Webrev: http://cr.openjdk.java.net/~vtewari/8165437/webrev0.5/index.html
>
> This change will replace the "gettimeofday" to use the monotonic
> increasing clock(i used the existing function "JVM_NanoTime" instead of
> writing my own).
>
> Thanks,
>
> Vyom
>
>
>

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

[Vyom Tewari]

On Wednesday 12 April 2017 11:05 PM, Vyom Tewari wrote:


Hi Roger,

thanks for review, please see my  comment inline.

Vyom


On Wednesday 12 April 2017 08:17 PM, Roger Riggs wrote:

Hi Vyom,

Thanks for taking this on.   I have a few comments and questions.

In aix_close.c line 547 The code for if (nanoTimeout >= 
NSEC_PER_MSEC) seems ineffective.


agreed, but this check was previously there(timeout > 0) and this 
check will prevent the additional call to 
"JVM_NanoTime/gettimeofday()" in modified code. I think  we leave this 
check as it is and put the corresponding else block. Please do let me 
know your thought on this.

i think you got it right, the "if" at line 547 is ineffective.
Vyom
The update of nanoTime at 549-550 ensures the timeout is > 
NSEC_PER_MSEC if it loops.
On the first pass through the code if nanoTimeout was < NSEC_PER_MSEC 
it would
poll with a timeout of zero which returns immediately, EINTR is not 
possible.


The comment at 546 would be inaccurate if nanoTimeout is too small, 
because it will loop again.


right, this is on wrong place it has to be in inner if block. I put 
this comment to avoid the confusion why "nanoTimeout < NET_NSEC_PER_MSEC".
I think the behavior would be the same if the extra condition at 547 
is removed.


Most of the other xxx_close.c classes have the same structure.

PlainSocketImpl.c has the same structure


The bsd_close.c had the same kind of before and after checking but 
did previously
and conservatively I'd keep that structure though I think it was 
ineffective in that case too.


Thanks, Roger




On 4/11/2017 9:04 AM, Vyom Tewari wrote:

Hi,

Please review the code change for below issue.

Bug: https://bugs.openjdk.java.net/browse/JDK-8165437

Webrev: 
http://cr.openjdk.java.net/~vtewari/8165437/webrev0.5/index.html


This change will replace the "gettimeofday" to use the monotonic 
increasing clock(i used the existing function "JVM_NanoTime" instead 
of writing my own).


Thanks,

Vyom

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

[Vyom Tewari]

Hi Roger,

thanks for review, please see my  comment inline.

Vyom


On Wednesday 12 April 2017 08:17 PM, Roger Riggs wrote:

Hi Vyom,

Thanks for taking this on.   I have a few comments and questions.

In aix_close.c line 547 The code for if (nanoTimeout >= NSEC_PER_MSEC) 
seems ineffective.


agreed, but this check was previously there(timeout > 0) and this check 
will prevent the additional call to "JVM_NanoTime/gettimeofday()" in 
modified code. I think  we leave this check as it is and put the 
corresponding else block. Please do let me know your thought on this.
The update of nanoTime at 549-550 ensures the timeout is > 
NSEC_PER_MSEC if it loops.
On the first pass through the code if nanoTimeout was < NSEC_PER_MSEC 
it would
poll with a timeout of zero which returns immediately, EINTR is not 
possible.


The comment at 546 would be inaccurate if nanoTimeout is too small, 
because it will loop again.


right, this is on wrong place it has to be in inner if block. I put this 
comment to avoid the confusion why "nanoTimeout < NET_NSEC_PER_MSEC".
I think the behavior would be the same if the extra condition at 547 
is removed.


Most of the other xxx_close.c classes have the same structure.

PlainSocketImpl.c has the same structure


The bsd_close.c had the same kind of before and after checking but did 
previously
and conservatively I'd keep that structure though I think it was 
ineffective in that case too.


Thanks, Roger




On 4/11/2017 9:04 AM, Vyom Tewari wrote:

Hi,

Please review the code change for below issue.

Bug:   https://bugs.openjdk.java.net/browse/JDK-8165437

Webrev: http://cr.openjdk.java.net/~vtewari/8165437/webrev0.5/index.html

This change will replace the "gettimeofday" to use the monotonic 
increasing clock(i used the existing function "JVM_NanoTime" instead 
of writing my own).


Thanks,

Vyom

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

[Roger Riggs]

Hi Vyom,

Thanks for taking this on.   I have a few comments and questions.

In aix_close.c line 547 The code for if (nanoTimeout >= NSEC_PER_MSEC) 
seems ineffective.


The update of nanoTime at 549-550 ensures the timeout is > NSEC_PER_MSEC 
if it loops.
On the first pass through the code if nanoTimeout was < NSEC_PER_MSEC it 
would
poll with a timeout of zero which returns immediately, EINTR is not 
possible.


The comment at 546 would be inaccurate if nanoTimeout is too small, 
because it will loop again.


I think the behavior would be the same if the extra condition at 547 is 
removed.


Most of the other xxx_close.c classes have the same structure.

PlainSocketImpl.c has the same structure


The bsd_close.c had the same kind of before and after checking but did 
previously
and conservatively I'd keep that structure though I think it was 
ineffective in that case too.


Thanks, Roger




On 4/11/2017 9:04 AM, Vyom Tewari wrote:

Hi,

Please review the code change for below issue.

Bug:   https://bugs.openjdk.java.net/browse/JDK-8165437

Webrev: http://cr.openjdk.java.net/~vtewari/8165437/webrev0.5/index.html

This change will replace the "gettimeofday" to use the monotonic 
increasing clock(i used the existing function "JVM_NanoTime" instead 
of writing my own).


Thanks,

Vyom

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

[Vyom Tewari]

Hi,

Please review the code change for below issue.

Bug:   https://bugs.openjdk.java.net/browse/JDK-8165437

Webrev: http://cr.openjdk.java.net/~vtewari/8165437/webrev0.5/index.html

This change will replace the "gettimeofday" to use the monotonic 
increasing clock(i used the existing function "JVM_NanoTime" instead of 
writing my own).


Thanks,

Vyom