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
