Improve performance of Parallel by overriding map2 to avoid tupling

[djspiewak]

This is a bit of low-hanging fruit, but we currently define Parallel[F] given GenSpawn[F, _] in terms of the following construction:

      final override def map2[A, B, Z](fa: ParallelF[F, A], fb: ParallelF[F, B])(
          f: (A, B) => Z): ParallelF[F, Z] =
        ParallelF(
          F.both(ParallelF.value(fa), ParallelF.value(fb)).map { case (a, b) => f(a, b) }
        )

That's cool but there's a lot of unnecessary boxing in here, both in both and in racePair underneath. We can actually do a lot better given GenConcurrent[F, _], which will be the case quite a lot of the time in practical code. However, rather than creating some sort of sneaky implicit prioritization thing, I think the right approach here is probably to add a mapBoth function (feel free to bikeshed the name) to GenSpawn, then override it in GenConcurrent with a more efficient implementation in terms of Fiber and Deferred.

Overall this probably sounds a lot more intimidating of an enhancement than it actually is. Tldr, add the following to GenSpawn:

def mapBoth[A, B, C](fa: F[A], fb: F[B])(f: (A, B) => F[C]): F[C] = ???

Delegate the implementation to both in GenSpawn, but then override that implementation in GenConcurrent with something implemented in terms of Fiber and Deferred. The results should be a noticeable improvement in performance of the par operators.

[manufacturist]

I'll give it a try 😄

[vasilmkd]

@djspiewak Why is the mapBoth mapping function specified as (A, B) => F[Z] and why not (A, B) => Z? I'm curious.

[vasilmkd]

Maybe I'm just being dense, but then, if (A, B) => Z is used, there is no difference with map2. I still fail to see how the boxing of one pair is not being reintroduced by the F[Z] in the end. Sorry again.

Are we sure that we can't just reimplement map2 for Parallel directly to be more performant?

[djspiewak]

Why is the mapBoth mapping function specified as (A, B) => F[Z] and why not (A, B) => Z? I'm curious.

An error on my part. :-) It really should be (A, B) => Z. You're correct btw that this is exactly map2. Or rather, it is parMap2, which is the difference. Also, as has been pointed out in Discord, since @manufacturist realized that this can be done without Deferred, we're free to push the more optimized implementation up into GenSpawn, which IMO would be a good thing to do.