Impact of code difference in Collection#contains() worth improving?

[Martin Buchholz]

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 <vitalyd at 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" <martinrb at 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.Zibis at 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.Zibis at cosoco.de <mailto:
> >> Ulf.Zibis at 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;
> >>                  }
> >>
> >>
> >>
> >
>
>