Optimise Apply[{ Either, \/ }].apply2

[fommil]

close #1956

Running this perf test

jsonformat/jmh:run -i 5 -wi 5 -f1 -t1 -w1 -r1 .*Geo.*decodeManual*

from https://gitlab.com/fommil/scalaz-deriving

BEFORE this change

Benchmark                               Mode  Cnt      Score     Error  Units
GeoJSONBenchmarks.decodeManualError    thrpt    5  25727.854 ± 263.755  ops/s
GeoJSONBenchmarks.decodeManualSuccess  thrpt    5  26943.126 ± 230.573  ops/s

AFTER this change

GeoJSONBenchmarks.decodeManualError    thrpt    5  70845.254 ± 890.444  ops/s
GeoJSONBenchmarks.decodeManualSuccess  thrpt    5  84736.783 ± 473.980  ops/s

and WITH custom short circution impl of traverseDisjunction from #1956 (which is used in the repo's master... back out that change to run the above tests)

GeoJSONBenchmarks.decodeManualError    thrpt    5  586733.762 ±  6389.296  ops/s
GeoJSONBenchmarks.decodeManualSuccess  thrpt    5  110961.246 ±  468.384  ops/s

async-profiler alloc tracing shows that the \/- allocations are quite heavy. I guess that's to be expected.

[jdegoes]

@fommil Awesome work. Would be great to do this for bind too.

[fommil]

@jdegoes done, that actually has a non-negligible impact on my perf tests which aren't even testing for this!

[info] Benchmark                               Mode  Cnt      Score     Error  Units
[info] GeoJSONBenchmarks.decodeManualError    thrpt    5  74902.151 ± 488.520  ops/s
[info] GeoJSONBenchmarks.decodeManualSuccess  thrpt    5  88754.010 ± 759.063  ops/s

[fommil]

https://typelevel.org/blog/2014/11/10/why_is_adt_pattern_matching_allowed.html#adts-are-ok-to-go suggests that these optimisations are unneeded. My perf tests suggest otherwise on 2.12...

[edmundnoble]

You have quite a few "optimizations" here, not just touching apply2. Is it clear which are actually responsible for speedup, and why?

[fommil]

@edmundnoble the numbers cited are just the \/.apply2, everything else seems to have improved things a little bit and are just opportunistic while I'm in the area.

[fommil]

faster because there is no check in the second case

[fommil]

@xuwei-k what do you think?

[hobwekiva]

write a tailrec function instead of using exceptions?

[hobwekiva]

Also map runs in the opposite direction:

   def map[B](f: A => B): IList[B] = reverse.reverseMap(f)

It starts from the end, so your traverse is not the same as the usual one.
^^ a perfect example why side-effects are evil ;)

[hobwekiva]

def traverse[E, A, B](l: List[A], f: A => E Either B): E Either List[B] = {
  @annotation.tailrec def go(l: List[A], r: List[B]): E Either List[B] =
  l match {
    case a :: as =>
      f(a) match {
        case Right(b) => go(as, b :: r)
        case Left(e) => Left(e)
      }
    case Nil => Right(r.reverse)
  }
  go(l, Nil)
}

println(
  traverse(
    List(1, 2, 3, 4), 
    (a : Int) => if (a % 2 == 0) Left(a) else Right(a)))

changes made