RFR: 8029795 : LinkedHashMap.getOrDefault() doesn't update access order. (was Why doesn't new Map methods generate entry accesses on LinkedHashMap?)

[Mike Duigou]

I've updated the documentation per Paul's suggestions. Specifically, in addition to the existing put() and get() methods the new Map methods

putIfAbsent()
getOrDefault()
compute()
computeIfAbsent()
computeIfPresent()
merge()

are all documented to perform a single access of the entry (assuming that an entry is created or modified as a result). 

replace()

is documented as accessing the entry, if it exists and a replacement is made. If no replacement is made then no access is recorded.

http://cr.openjdk.java.net/~mduigou/JDK-8029795/2/webrev/

Mike


On Dec 11 2013, at 02:32 , Paul Sandoz <paul.sandoz at oracle.com> wrote:

> 
> On Dec 11, 2013, at 1:27 AM, Mike Duigou <mike.duigou at oracle.com> wrote:
> 
>> I have added tests and documentation for the other methods.
>> 
>> http://cr.openjdk.java.net/~mduigou/JDK-8029795/1/webrev/
>> 
>> The documentation for some of the methods is ambiguous about how many access events are generated. For LRU cache is OK but other cases (counting based eviction) may care about the total number of accesses.
> 
> + * invocation completes).  Invoking the {@code replace}, {@code merge},
> + * {@code compute} or {@code computeIfPresent} methods results in one or more
> + * accesses of the corresponding entry. The {@code putAll} method generates one
> 
> If i look at the code for say replace it generates *at most one access* in terms of affecting last access order, if the entry exists and if the old/new value constraint holds:
> 
>    @Override
>    public boolean replace(K key, V oldValue, V newValue) {
>        Node<K,V> e; V v;
>        if ((e = getNode(hash(key), key)) != null &&
>            ((v = e.value) == oldValue || (v != null && v.equals(oldValue)))) {
>            e.value = newValue;
>            afterNodeAccess(e);
>            return true;
>        }
>        return false;
>    }
> 
> 
> I have attempted to categorise the access behaviour of each method:
> 
> get:
>  access to entry, if it exists
> 
> put:
>  access to new entry
> 
> putIfAbsent
>  access to entry, if it exists
>  otherwise access to new entry
> 
> replace(K key, V value)
>  access to entry, if it exists
> 
> replace(K key, V oldValue, V newValue)
>  access to entry, if it exists and it's value is updated
> 
> computeIfAbsent:
>  access to entry, if it exists and it's value is non-null, or 
>  access to entry, if it exists and it's null value is updated to a non-null value [*], 
>  otherwise access to new entry, if created
> 
> computeIfPresent
>  access to entry, if it exists and it's non-null value is updated to a non-null value
> 
> compute
>  access to entry, if it exists and it's value is updated to a non-null value,
>  otherwise access to new entry, if created
> 
> merge
>  access to entry, if it exists and it's value is updated to a non-null value,
>  otherwise access to new entry, if created
> 
> 
> Patterns, to help group for documentation:
> 
> get/put/putIfAbsent/replace(K key, V value): access to entry associated with the key, if entry exists after invocation completes
> 
> replace(K key, V oldValue, V newValue): access to entry associated with the key, if returns true
> 
> computeIfAbsent/computeIfPresent/compute/merge: access to entry associated with the key, if returns a non-null value.
> 
> 
> --
> 
> [*] There is some ambiguity with computeIfAbsent. When the entry exists, it's value is null, and the value returned from the mapping function is null then, an access to that entry occurs:
> 
>        V v = mappingFunction.apply(key);
>        if (old != null) {
>            old.value = v;
>            afterNodeAccess(old);
>            return v;
>        }
>        else if (v == null)
>            return null;
> 
> Is that a bug? I would have expected the returning of null from the mapping function to signal no-action to be taken. Thus computeIfPresent and computeIfAbsent have no side-effects for an existing entry whose value is null when the function returns null (same applies to merge, if the value parameter is null or the remapping function returns null, and to compute, if the remapping function returns null).
> 
> So perhaps that code should be:
> 
>        V v = mappingFunction.apply(key);
>        if (v == null)
>           return null;
>        else if (old != null) {
>            old.value = v;
>            afterNodeAccess(old);
>            return v;
>        }
> 
> Paul.