RFR [9] 8177536: Avoid Apple Peer-to-Peer interfaces in networking tests

[Chris Hegarty]

This is a tests only change.

Several networking tests appear to fail intermittently on recent
Mac machines. In most cases an Apple Peer-to-Peer, awdl0,
interface, has been seen to be chosen by the test, for some
operation or other, when enumerating the set of network
interfaces on the machine. Such an interface may not be
suitable to perform the operation required by the test, and
thus best avoided.

http://cr.openjdk.java.net/~chegar/8177536.00/ 


This change attempts to create a NetworkConfiguration utility
class that can be expanded on to centralize the logic for most
networking related tests. For now, I’ve done the minimum
possible here to get the tests back to a reasonable shape, but
I expect that we can do further clean up and consolidation in the
future.

-Chris.

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

Re: RFR [9] 8177536: Avoid Apple Peer-to-Peer interfaces in networking tests

[Roger Riggs]

Hi Chris,

Looks good, nice use of streams.

B6558853.java: 28:  an extra "*"

A few tests do nothing unless an interface is found; they could use 
either output that says what

interface is tested or a message that no interface was available.
(In case someone is reviewing the logs and wants to know what was tested).

Roger



On 4/12/2017 9:56 AM, Chris Hegarty wrote:

This is a tests only change.

Several networking tests appear to fail intermittently on recent
Mac machines. In most cases an Apple Peer-to-Peer, awdl0,
interface, has been seen to be chosen by the test, for some
operation or other, when enumerating the set of network
interfaces on the machine. Such an interface may not be
suitable to perform the operation required by the test, and
thus best avoided.

http://cr.openjdk.java.net/~chegar/8177536.00/ 



This change attempts to create a NetworkConfiguration utility
class that can be expanded on to centralize the logic for most
networking related tests. For now, I’ve done the minimum
possible here to get the tests back to a reasonable shape, but
I expect that we can do further clean up and consolidation in the
future.

-Chris.

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

[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