Refactoring the hierarchy should be separate (and much more controversial) change. This change is a clean win, just very small. Refactoring runs into space/time tradeoffs.
On Fri, Aug 29, 2014 at 3:23 PM, Vitaly Davidovich <vita...@gmail.com> wrote: > :) so if you're going to do this, is there no base class in the hierarchy > where this can be placed (I don't have source in front of me)? That way > there's a higher likelihood that the pattern will stay consistent (with new > implementations at least). > > Sent from my phone > On Aug 29, 2014 5:56 PM, "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; > >> } > >> > >> > >> > > > >
