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

[Mike Duigou]

I considered it but was worried that the combined size would prevent inlining of the method. The getOrDefault method is currently 33 bytes long which is right at the common borderline for inlining. It does seem a shame to duplicate the code but I couldn't be confident I wouldn't degrade performance by reuse. 

(If someone were to write a JMH benchmark to show that get() calling getOrDefault() was no slower than the naive duplication then it would be reasonable to consider switching).

Mike

On Dec 9 2013, at 23:22 , Vitaly Davidovich <vitalyd at gmail.com> wrote:

> Mike,
> 
> Would it make sense to rewrite get() in terms of getOrDefault() to reduce duplication?
> 
> Thanks
> 
> Sent from my phone
> 
> On Dec 9, 2013 10:40 PM, "Mike Duigou" <mike.duigou at oracle.com> wrote:
> Hello all;
> 
> I've posted a webrev for review which corrects the problem and adds appropriate tests.
> 
> http://cr.openjdk.java.net/~mduigou/JDK-8029795/0/webrev/
> 
> I also updated the documentation to mention that getOrDefault as well as the replace methods generate access events.
> 
> Mike
> 
> On Dec 9 2013, at 02:11 , Paul Sandoz <paul.sandoz at oracle.com> wrote:
> 
> > Hi Roman,
> >
> > On Dec 8, 2013, at 10:29 PM, Roman Leventov <leventov at ya.ru> wrote:
> >> Especially getDefault(). Doesn't this violate principle of least astonishment? Details and proof: http://stackoverflow.com/questions/20440136/why-doesnt-new-map-methods-generate-entry-accesses-on-linkedhashmap
> >>
> >
> > Thanks. I believe that all the new (default) Map methods but getOrDefault behave correctly. So i think it is a bug, we need to add something like the following to LinkedHashMap:
> >
> >    public V getOrDefault(Object key, V defaultValue) {
> >        Node<K,V> e;
> >        if ((e = getNode(hash(key), key)) == null)
> >            return defaultValue;
> >        if (accessOrder)
> >            afterNodeAccess(e);
> >        return e.value;
> >    }
> >
> > and also update the documentation to clarify (via the implementation specification of the default methods it can be inferred what the behaviour is, but that ain't obvious).
> >
> > I have logged this bug:
> >
> >  https://bugs.openjdk.java.net/browse/JDK-8029795
> >
> > Paul.
> >
> >
> >
>