On 07/01/2014 10:04 PM, Martin Buchholz wrote:
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.
I have filed a RFE that is more suitable for this change:
https://bugs.openjdk.java.net/browse/JDK-8049228
So, is it strictly necessary to fix JDK-7186258 alone before applying
the change for JDK-8049228 although it would supersede it?
Regards, Peter
IIRC lookup.sh is a known flaky test being fixed elsewhere.
On Tue, Jul 1, 2014 at 11:35 AM, Peter Levart <peter.lev...@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
