A few more chain optimizations

[johnynek]

cc @bplommer

1. reorganize some matches in order I guess they are more likely to match
2. tighten the type on some loops so we don't have to check against Empty
3. implement foldLeft the same way as length (this was not a benchmark win).
4. override foldMap to use the iterator (to leverage Monoid.combineAll).

[johnynek]

by passing the destructured list, we avoid an allocation in the Append case (which is common since Append is the way we combine two chains).

[satorg]

If I am not missing something, then if (acc <= 0L) 1 should work as well.
Because we know that the head is not empty, so seems we can skip looking further if acc == 0 is given.

[johnynek]

yep. Good call. Updated.

[bplommer]

Would there be any benefit to leaving these abstract in Chain and implementing them separately in Chain.Empty and Chain.NonEmpty?

[johnynek]

I ran the benchmarks (and added two). Here are the relevant numbers:

PR:

[info] Benchmark                                  Mode  Cnt          Score           Error  Units
[info] ChainBench.foldLeftLargeChain             thrpt    5         54.823 ±        14.480  ops/s
[info] ChainBench.foldLeftLargeList              thrpt    5        152.206 ±         5.792  ops/s
[info] ChainBench.foldLeftSmallChain             thrpt    5   74133841.930 ±    400335.145  ops/s
[info] ChainBench.foldLeftSmallList              thrpt    5   81573852.962 ±    281124.077  ops/s
[info] ChainBench.lengthLargeChain               thrpt    5     208482.471 ±      1696.443  ops/s
[info] ChainBench.lengthLargeList                thrpt    5        529.225 ±         0.394  ops/s
[info] ChainBench.reverseLargeChain              thrpt    5      12154.755 ±      3341.891  ops/s
[info] ChainBench.reverseLargeList               thrpt    5        131.259 ±       169.766  ops/s

main:
[info] Benchmark                                  Mode  Cnt          Score          Error  Units
[info] ChainBench.foldLeftLargeChain             thrpt    5        132.305 ±        3.237  ops/s
[info] ChainBench.foldLeftLargeList              thrpt    5        152.748 ±        3.895  ops/s
[info] ChainBench.foldLeftSmallChain             thrpt    5   76658960.380 ±   237704.833  ops/s
[info] ChainBench.foldLeftSmallList              thrpt    5   81733659.606 ±    45817.907  ops/s
[info] ChainBench.lengthLargeChain               thrpt    5      12564.835 ±    21329.022  ops/s
[info] ChainBench.lengthLargeList                thrpt    5        536.412 ±       10.714  ops/s
[info] ChainBench.reverseLargeChain              thrpt    5        175.292 ±       49.151  ops/s
[info] ChainBench.reverseLargeList               thrpt    5        174.277 ±       21.204  ops/s

so it looks like the reverse and length changes are big wins, but the foldLeft change is not an improvement. I'll revert that part.

[satorg]

Looks really cool, thanks!

[satorg]

Would there be any benefit to leaving these abstract in Chain and implementing them separately in Chain.Empty and Chain.NonEmpty?

AFAIU, implementing all the logic in an abstract class rather than spreading it among descendants may benefit from code locality i.e. it should be less likely that an instruction cache will be reloading while executing a method of the abstract class. Perhaps @johnynek may correct me if I'm wrong about that.

[johnynek]

I really don't know. Of course experiments are the best way to find out but even our current benchmarks might steer us wrong depending on the frequency of Empty, frequency or wrapping and structure of concatenation.