RFR JDK-7186258: InetAddress$Cache should replace currentTimeMillis with nanoTime (+more)

[Martin Buchholz]

I haven't looked at the patch, but generally ... all uses of
currentTimeMillis to measure elapsed time should be migrated to use
difference of two nanoTime values, and such a change should be done
independently of other changes.

IIRC lookup.sh is a known flaky test being fixed elsewhere.


On Tue, Jul 1, 2014 at 11:35 AM, Peter Levart <peter.levart at gmail.com>
wrote:

>  Hi,
>
> I propose a patch for this issue:
>
>     https://bugs.openjdk.java.net/browse/JDK-7186258
>
> The motivation to re-design caching of InetAddress-es was not this issue
> though, but a desire to attack synchronization bottlenecks in methods like
> URL.equals and URL.hashCode which use host name to IP address mapping. I
> plan to tackle the synchronization in URL in a follow-up proposal, but I
> wanted to 1st iron-out the "leaves" of the call-tree. Here's the proposed
> patch:
>
> http://cr.openjdk.java.net/~plevart/jdk9-dev/InetAddress.Cache/webrev.01/
>
> sun.net.InetAddressCachePolicy:
>
> - two static methods (get() and getNegative()) were synchronized. Removed
> synchronization and made underlying fields volatile.
> - also added a normalization of negative policy in setNegativeIfNotSet().
> The logic in InetAddress doesn't cope with negative values distinct from
> InetAddressCachePolicy.FOREVER (-1), so this was a straight bug. The
> setIfNotSet() doesn't need this normalization, because checkValue() throws
> exception if passed-in value < InetAddressCachePolicy.FOREVER.
>
> java.net.InetAddress:
>
> - complete redesign of caching. Instead of distinct Positive/Negative
> caches, there's only one cache - a ConcurrentHashMap. The value in the map
> knows if it contains positive or negative answer.
> - the design of this cache is similar but much simpler than
> java.lang.reflect.WeakCache, since it doesn't have to deal with
> WeakReferences and keys are simpler (just strings - hostnames). Similarity
> is in how concurrent requests for the same key (hostname) are synchronized
> when the entry is not cached yet, but still avoid synchronization when
> entry is cached. This preserves the behaviour of original InetAddress
> caching code but simplifies it greatly (100+ lines removed).
> - I tried to preserve the interaction between InetAddress.getLocalHost()
> and InetAddress.getByName(). The getLocalHost() caches the local host
> address for 5 seconds privately. When it expires it performs new name
> service look-up and "refreshes" the entry in the InetAddress.getByName()
> cache although it has not expired yet. I think this is meant to prevent
> surprises when getLocalHost() returns newer address than getByName() which
> is called after that.
> - I also fixed the JDK-7186258 as a by-product (but don't know yet how to
> write a test for this issue - any ideas?)
>
> I created a JMH benchmark that tests the following methods:
>
> - InetAddress.getLocalHost()
> - InetAddress.getByName() (with positive and negative answer)
>
> Here're the results of running on my 4-core (8-threads) i7/Linux:
>
>
> http://cr.openjdk.java.net/~plevart/jdk9-dev/InetAddress.Cache/InetAddress.Cache_bench_results.01.pdf
>
> The getByNameNegative() test does not show much improvement in patched vs.
> original code. That's because by default the policy is to NOT cache
> negative answers. Requests for same hostname to the NameService(s) are
> synchronized. If "networkaddress.cache.negative.ttl" system property is set
> to some positive value, results are similar to those of getByNamePositive()
> test (the default policy for positive caching is 30 seconds).
>
> I ran the jtreg tests in test/java/net and have the same score as with
> original unpatched code. I have 3 failing tests from original and patched
> runs:
>
> JT Harness : Tests that failed
> java/net/MulticastSocket/Promiscuous.java: Test for interference when two
> sockets are bound to the same port but joined to different multicast groups
> java/net/MulticastSocket/SetLoopbackMode.java: Test
> MulticastSocket.setLoopbackMode
> java/net/MulticastSocket/Test.java: IPv4 and IPv6 multicasting broken on
> Linux
>
> And 1 test that had error trying to be run:
>
> JT Harness : Tests that had errors
> java/net/URLPermission/nstest/lookup.sh:
>
> Because of:
>
> test result: Error. Can't find source file: jdk/testlibrary/*.java in
> directory-list:
> /home/peter/work/hg/jdk9-dev/jdk/test/java/net/URLPermission/nstest
> /home/peter/work/hg/jdk9-dev/jdk/test/lib/testlibrary
>
> All other 258 java/net tests pass.
>
>
>
> So what do you think?
>
>
> Regards, Peter
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20140701/c5708905/attachment-0001.html>