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

[Martin Buchholz]

In java.util, we're willing to take maintainability hits for the sake of
performance.  OTOH, this is the part of the JDK most likely to be studied
by students everywhere, so exemplary code is also highly desirable.


On Tue, Dec 10, 2013 at 8:43 AM, Mike Duigou <mike.duigou at oracle.com> wrote:

> 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.
> > >
> > >
> > >
> >
>
>