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

[Ulf Zibis]

Hi Martin,

why you didn't include Mike's suggestion into the fix?

-Ulf

Am 02.09.2014 um 19:04 schrieb Mike Duigou:
> 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 <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;
>>>>                  }
>>>>
>>>>
>>>>
>