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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20170412/8409d8cf/attachment.html>