I believe C2 compiler is more aggressive about inlining, and will inline hot methods over the 35 bytes limit. I guess interpreter and C1 (and other JVMs, potentially) may get impacted though. In this case, the issue may be more about the fact that getOrDefault() is virtual and not so much about byte code size; devirtualization will need profiling info. Also, the inlining method (I.e. get()) would be tiny since it just calls getOrDefault - if inlining is halted purely on byte code size, that almost smells of a compiler bug/quality of implementation issue.
Having said that, it's really a shame that such a seemingly small (and good, from maintenance standpoint) change risks performance degradation. Sent from my phone On Dec 10, 2013 10:44 AM, "Mike Duigou" <mike.dui...@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 <vita...@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.dui...@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.san...@oracle.com> wrote: > > > Hi Roman, > > > > On Dec 8, 2013, at 10:29 PM, Roman Leventov <leven...@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. > > > > > > > >
