Avoid unnecessary flatMaps across all execution phases

[kyri-petrou]

This PR aims at improving performance by reducing the number of ZIO / ZPure flatMaps during the lifecycle of a GraphQL request. While the cost of extra flatmaps is generally amortized in large queries, they have a non-negligible impact on very simple queries.

Validation:

• For simple validations where we don't need to return a ZPure (i.e., when the method is already stack-safe), return an Option instead and don't flatmap on it if it's None
• Optimize collection of directives, selection sets and fragments
• Use ZPure.environmentWithPure instead of ZPure.serviceWithPure. This is very surprising to me - The performance of environmentWithPure is much better than serviceWithPure, even if they're implemented in the same way. Looking at the benchmark CPU profiles, there seems to be something really dodgy going on with serviceWithPure and the implicit tag requirement. I need to investigate this further, I think it's a bug in zio-prelude

VariableCoerser:

• Add coerceVariablesEither method and shortcut to Right(Map.empty) if no variables were passed

GraphQL (considering to revert this if we don't think it's needed):

• Break down the executeRequest into separate methods, each representing an execution phase. This isn't much of an optimization, mostly a bit of a cleanup so that we don't accidentally use any of the ExecutionConfiguration values obtained from a previous phase, which might have been mutated within the wrappers

[ghostdogpr]

The old code was reading the configuration only once for skipValidation and enableIntrospection. Not sure how much it matters.

[kyri-petrou]

For skipValidation, it was actually reading it twice (once before coerceVariables and another before Validator.prepare which is the intended behaviour as Validation wrappers might change the skipValidation value.

This was one of the reasons I thought it'd be good to separate the phases into separate methods as well, because it felt like a bug waiting to happen if we accidentally read the state prior to the validation wrappers being applied.

As for isIntrospection, since that's a lazy val on Document, the cost of reading it twice is negligible

[ghostdogpr]

ZIO.fromEither?

[kyri-petrou]

ZIO.fromEither is implemented as ZIO.succeed(either).map(_.fold(ZIO.fail(_), ZIO.succeed(_)) which is 3 flatMaps instead of 1 (note that ZIO's .map is also a flatMap). It's a very micro-optimization, so I'm happy to revert it, but I thought since VariablesCoercer.coerceVariablesEither is not side-effecting might as well avoid the extra flatmaps

[paulpdaniels]

I need to investigate this further, I think it's a bug in zio-prelude

I wonder if it has to do with the double indirection through two partially applied classes

serviceWithPure => ServiceWithPurePartiallyApplied => environmentWithPure => EnvironmentWithPartiallyApplied

I know there are certain cases where AnyVal can still cause boxing with the JVM. Could this be one of them?

[kyri-petrou]

I know there are certain cases where AnyVal can still cause boxing with the JVM. Could this be one of them?

@paulpdaniels Not sure what it is, but the difference in profiles is very bizarre. Given this benchmark:

package example.benchmarks

import org.openjdk.jmh.annotations.*
import zio.prelude.fx.ZPure

import java.util
import java.util.concurrent.TimeUnit

@State(Scope.Thread)
@BenchmarkMode(Array(Mode.AverageTime))
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS)
@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
@Fork(1)
@Threads(8)
class Testy2Benchmark {

  case class Foo2(v: String)

  @Benchmark
  def bench1 =
    ZPure
      .collectAll(List.fill(100)(ZPure.serviceWithPure[Foo2](f => ZPure.succeed(f.v))))
      .provideService(Foo2("foo"))
      .runEither

  @Benchmark
  def bench2 =
    ZPure
      .collectAll(List.fill(100)(ZPure.environmentWithPure[Foo2](env => ZPure.succeed(env.get[Foo2].v))))
      .provideService(Foo2("foo"))
      .runEither
}

These are the profiles:

Bench1:

Bench2:

[kyri-petrou]

@ghostdogpr this one doesn't contain bin-incompatible changes so I'll merge it in