Looks fine (bug updated with noreg-perf). I do think we should consider promoting a copy of this method to AbstractList as it generally allows for a much better implementation than AbstractCollection's contains().
Mike On Aug 29 2014, at 14:56 , Martin Buchholz <marti...@google.com> wrote: > Just think - one whole byte saved for each individual change! > I have a webrev! > http://cr.openjdk.java.net/~martin/webrevs/openjdk9/pico-optimize-contains/ > https://bugs.openjdk.java.net/browse/JDK-8056951 > > Can haz review please? > > > On Fri, Aug 29, 2014 at 1:05 PM, Ulf Zibis <ulf.zi...@cosoco.de> wrote: > >> >> Am 28.08.2014 um 19:46 schrieb Vitaly Davidovich: >> >> >>> There's no register pressure - the immediate (I.e. -1) is encoded >>> directly into the instruction, just like 0 would be. The time when 0 is >>> particularly useful is when you test for it in the zero bit of the flags >>> register (e.g. dec followed by jz, such as when counting a loop down to >>> 0). Otherwise, I don't see any advantage from machine code perspective. >>> >>> >> Thanks for explaining this, but a very little nit: the immediate (I.e. -1) >> uses additional 32/64 bits in code which must be loaded from memory and >> wastes space in CPU cache or am I wrong? This could be saved with >= 0. >> >> So if unifying the code I agree to Martin's opinion. >> >> -Ulf >> >> The aforementioned cmov instruction is not without its own downsides, so >>> it's unclear which is better when branch probability isn't known a priori. >>> >>> The 1 byte code is unlikely to make any difference, unless jit is turned >>> off and you're running this through a tight loop in the interpreter (but if >>> one does that, perf conversation is moot :)). >>> >>> Sent from my phone >>> >>> On Aug 28, 2014 1:28 PM, "Ulf Zibis" <ulf.zi...@cosoco.de <mailto: >>> ulf.zi...@cosoco.de>> wrote: >>> >>> >>> Am 27.08.2014 um 17:51 schrieb Martin Buchholz: >>> >>> The ArrayList version saves one byte of bytecode, and is >>> therefore very >>> slightly better. We should bless that version and use it >>> consistently. >>> >>> >>> +1 >>> Additional argument: >>> The LinkedList code requires to load 32/64-Bit -1 into CPU. This may >>> take some time on some >>> CPU and at least wastes memory footprint. >>> Additionally register pressure increases. >>> Vitaly, please correct me, if I'm wrong, just for learning more. >>> >>> Another advantage is that there is no problem if some implementation >>> of indexOf() erroneously >>> returns another negative value than -1. I remember some compare() >>> implementations, which >>> sometimes return different values than only -1, 0, +1. >>> >>> -Ulf >>> >>> ArrayList: >>> >>> public boolean contains(Object o) { >>> return indexOf(o) >= 0; >>> } >>> >>> LinkedList: >>> >>> public boolean contains(Object o) { >>> return indexOf(o) != -1; >>> } >>> >>> >>> >>
