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

[Peter Levart]

On 07/02/2014 12:58 AM, Bernd Eckenfels wrote:
> Hello,
>
> Am Wed, 02 Jul 2014 00:45:01 +0200
> schrieb Peter Levart <peter.levart at gmail.com>:
>>> L782: is it better to use putIfAbsent unconditionally, instead of
>>> get/putIfAbsent in NameServicdeAddr?
>> I want to keep the semantics of original code that guarantees that
>> there will only be a single look-up to the name service per hostname
>> in a given caching period. If several threads want to look-up the
>> same hostname at the same time, only one will do it, others will wait
>> and then use the result of that thread. That's why I re-check the
>> cached value after entering the synchronized block and act upon what
>> I find there...
> Yes, I was talking about the get inside the synchronized with same
> semantic, but I havent completely tested my thinking through (idea was
> to re-use putIfAbsent for the first get):
>
> synchronized (this) {
>    // re-check that we are still us
>   addresses = cache.putIfAbsent(host, this);
>   // address is null if we had added us
>   // if address did not change it is this

You're right. A single putIfAbsent can do it. Thanks for pointing that out.

Regards, Peter

>
>
>
>>> L732: I am unsure about the id field, isnt it enough to have the
>>> identity equality check for the replacement check and otherwise
>>> depend on equals()?
>> The ConcurrentSkipListSet (based on ConcurrentSkipListMap) that is
>> used as an ordered concurrent (but not blocking or locking) queue of
>> items to be expired is not using hashCode/equals, but compareTo
>> instead. So two instances of CachedAddresses must never compareTo
>> each other as being "equal" if one wants to keep them all in the
>> ConcurrentSkipListSet...
> Yes, I mean not using id in compareTo but idendity if (other==this)
> return 0; But maybe not worth it as you still need to order identical
> entries reliable (maybe by system hash).
>
>>> BTW1: might be the wrong RFR, but considering your good performance
>>> numbers for an active cache, would having 100ms or similiar default
>>> negative cache time make sense without impacting (visible) the
>>> semantic.
>> It depends on the "application". If some app is doing more than 10
>> look-ups of same non-existent hostname per second, it will have
>> effect. Otherwise not. It seems that negative answers are not so
>> frequent in practice that default policy would require caching them.
>> One can always choose the policy manually.
> Actually I think an application who retries lookups without rate limit
> might not be too uncommon as they typically fail only after timeouts.
> However if you have for example a dropped default route nameserver
> answer can return the error immediatelly and you get into a busy loop.
> But yes most likely most apps only try it a small time.
>
> Gruss
> Bernd