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 <[email protected]> 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" <[email protected]> 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 <[email protected]> wrote: > > > Hi Roman, > > > > On Dec 8, 2013, at 10:29 PM, Roman Leventov <[email protected]> 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. > > > > > > >
