Optimize Parallel instance in GenSpawnInstances to avoid allocation.

[nmichael44]

Motivation

Currently in GenSpawnInstances, the Parallel instance provides .parallel and .sequential as defs that instantiate a new (M ~> F) and new (F ~> M) on every invocation.

Because operations like cats.Parallel.parTraverse invoke P.parallel(ma) and P.sequential(ma) for every item in the collection, this creates a large number of FunctionK object allocations during a traversal, putting unnecessary pressure on the garbage collector.

Changes

This PR eliminates these allocations by applying the Id caching technique (similar to Resource.liftK).

• Extracts the FunctionK instances into a private object GenSpawnInstances to act as true JVM-wide singletons (rather than trait fields).
• Uses asInstanceOf to cast the cached Id transformations to the required effect type at the call site.

Impact

Zero-allocation parallel and sequential transformations for all effect types that implement GenSpawn. This reduces the memory footprint and GC overhead on hot paths involving parallel collections processing.

[nmichael44]

Hi @durban, maybe you could review this one as well? In the same spirit as another one you did recently. There is also one more just like it right next to this one.

[durban]

I'm slightly nervous about the .asInstanceOf being "far away" from what we're casting. (In general, I deem .asInstanceOf a code smell. It is correct here, but not obviously correct. The PR I've merged had the cast in the vicinity of the definition, so it was obvious.)

Have you observed the reduced memory footprint and GC overhead you write about? (I believe we have parTraverse benchmarks, for example.)

[nmichael44]

@durban I did not observe reduced memory -- I wasn't trying to solve a performance problem. I found these by reading the code of cats effect while trying to understand the framework better. Of all the ones you looked this is the one that could have the most impact since we build a natural transform for each element in the collection. But it's up to you. Maybe if I add some comment above the reference to the constant natural transform that would help convince you?

[durban]

Let's move the .asInstanceOf right next to the cachedSequential into a private method, so it's obvious what's being casted, and that it is correct (and similarly for cachedParallel). And then the fields can be private[this], and only accessed through the methods.

[nmichael44]

@durban I made the changes you asked for (hopefully). Please take a look. We make 2 calls now instead of one when we call those functions but hopefully the magic of inlining will kick in and save the day.

[durban]

Thanks. Yes, I reckon a method call should be much-much cheaper than an allocation. As you say, inlining should take care of it.