From cac16ec4505b81e82e6c0a7f23dd1cdd1afc90c2 Mon Sep 17 00:00:00 2001 From: Kyri Petrou Date: Fri, 14 Jun 2024 19:21:59 +1000 Subject: [PATCH 1/2] Optimize `Field` creation when selection set doesn't contain fragments --- .../validation/ValidationBenchmark.scala | 74 +++++++++++-- .../main/scala/caliban/execution/Field.scala | 104 ++++++++++++------ 2 files changed, 134 insertions(+), 44 deletions(-) diff --git a/benchmarks/src/main/scala/caliban/validation/ValidationBenchmark.scala b/benchmarks/src/main/scala/caliban/validation/ValidationBenchmark.scala index 2e31d5784..9829494ac 100644 --- a/benchmarks/src/main/scala/caliban/validation/ValidationBenchmark.scala +++ b/benchmarks/src/main/scala/caliban/validation/ValidationBenchmark.scala @@ -4,7 +4,7 @@ import caliban._ import caliban.execution.NestedZQueryBenchmarkSchema import caliban.introspection.Introspector import caliban.parsing.{ Parser, VariablesCoercer } -import caliban.schema.RootType +import caliban.schema.{ RootSchema, RootType } import org.openjdk.jmh.annotations.{ Scope, _ } import zio._ @@ -15,15 +15,15 @@ import java.util.concurrent.TimeUnit @OutputTimeUnit(TimeUnit.SECONDS) @Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS) @Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS) -@Fork(1) +@Fork(2) class ValidationBenchmark { private val runtime = Runtime.default - def run[A](zio: Task[A]): A = Unsafe.unsafe(implicit u => runtime.unsafe.run(zio).getOrThrow()) - def toSchema[R](graphQL: GraphQL[R]): IO[CalibanError, RootType] = + def run[A](zio: Task[A]): A = Unsafe.unsafe(implicit u => runtime.unsafe.run(zio).getOrThrow()) + def toSchema[R](graphQL: GraphQL[R]): IO[CalibanError, (RootSchema[R], RootType)] = graphQL.validateRootSchema.map { schema => - RootType( + schema -> RootType( schema.query.opType, schema.mutation.map(_.opType), schema.subscription.map(_.opType) @@ -38,11 +38,11 @@ class ValidationBenchmark { val parsedDeepWithArgsQuery = run(Parser.parseQuery(deepWithArgsQuery)) val parsedIntrospectionQuery = run(Parser.parseQuery(ComplexQueryBenchmark.fullIntrospectionQuery)) - val simpleType = run( + val (simpleSchema, simpleType) = run( toSchema(graphQL[Any, SimpleRoot, Unit, Unit](RootResolver(NestedZQueryBenchmarkSchema.simple100Elements))) ) - val multifieldType = + val (multifieldSchema, multifieldType) = run( toSchema( graphQL[Any, MultifieldRoot, Unit, Unit]( @@ -51,7 +51,7 @@ class ValidationBenchmark { ) ) - val deepType = + val (deepSchema, deepType) = run( toSchema( graphQL[Any, DeepRoot, Unit, Unit]( @@ -60,7 +60,7 @@ class ValidationBenchmark { ) ) - val deepWithArgsType = + val (deepWithArgsSchema, deepWithArgsType) = run( toSchema( graphQL[Any, DeepWithArgsRoot, Unit, Unit]( @@ -100,4 +100,60 @@ class ValidationBenchmark { run(io) } + @Benchmark + def fieldCreationSimple(): Any = + Validator + .prepareEither( + parsedSimpleQuery, + simpleType, + simpleSchema, + None, + Map.empty, + skipValidation = true, + validations = Nil + ) + .fold(throw _, identity) + + @Benchmark + def fieldCreationMultifield(): Any = + Validator + .prepareEither( + parsedMultifieldQuery, + multifieldType, + multifieldSchema, + None, + Map.empty, + skipValidation = true, + validations = Nil + ) + .fold(throw _, identity) + + @Benchmark + def fieldCreationDeep(): Any = + Validator + .prepareEither( + parsedDeepQuery, + deepType, + deepSchema, + None, + Map.empty, + skipValidation = true, + validations = Nil + ) + .fold(throw _, identity) + + @Benchmark + def fieldCreationIntrospection(): Any = + Validator + .prepareEither( + parsedIntrospectionQuery, + Introspector.introspectionRootType, + simpleSchema, + None, + Map.empty, + skipValidation = true, + validations = Nil + ) + .fold(throw _, identity) + } diff --git a/core/src/main/scala/caliban/execution/Field.scala b/core/src/main/scala/caliban/execution/Field.scala index ab6ed05d3..dc6c9066e 100644 --- a/core/src/main/scala/caliban/execution/Field.scala +++ b/core/src/main/scala/caliban/execution/Field.scala @@ -11,6 +11,7 @@ import caliban.schema.{ RootType, Types } import caliban.{ InputValue, Value } import scala.collection.mutable +import scala.collection.mutable.ListBuffer /** * Represents a field used during the execution of a query @@ -135,7 +136,7 @@ object Field { directives: List[Directive], rootType: RootType ): Field = { - val memoizedFragments = new mutable.HashMap[String, (List[(Field, Option[String])])]() + val memoizedFragments = new mutable.HashMap[String, List[(Field, Option[String])]]() val variableDefinitionsMap = variableDefinitions.map(v => v.name -> v).toMap def loop( @@ -144,12 +145,8 @@ object Field { fragment: Option[Fragment], targets: Option[Set[String]], condition: Option[Set[String]] - ): List[(Field, Option[String])] = { - val map = new java.util.LinkedHashMap[(String, Option[String]), Field]() - - def addField(f: Field, condition: Option[String]): Unit = - map.compute((f.aliasedName, condition), (_, existing) => if (existing == null) f else existing.combine(f)) - + ): Either[List[Field], List[(Field, Option[String])]] = { + val builder = FieldBuilder.forSelectionSet(selectionSet) val innerType = fieldType.innerType selectionSet.foreach { @@ -166,15 +163,15 @@ object Field { val fields = if (selectionSet.nonEmpty) loop(selectionSet, t, None, None, None) - else Nil // Fragments apply on to the direct children of the fragment spread + else leftNil // Fragments apply on to the direct children of the fragment spread - addField( + builder.addField( new Field( name, t, Some(innerType), alias, - fields.map(_._1), + fields.fold(identityFnList, _.map(_._1)), targets = targets, arguments = resolveVariables(arguments, variableDefinitionsMap, variableValues), directives = resolvedDirectives, @@ -189,28 +186,30 @@ object Field { val fields = memoizedFragments.getOrElseUpdate( name, { val resolvedDirectives = directives.map(resolveDirectiveVariables(variableValues, variableDefinitionsMap)) - val _fields = if (checkDirectives(resolvedDirectives)) { - fragments.get(name).map { f => - val typeCondName = f.typeCondition.name - val t = rootType.types.getOrElse(typeCondName, fieldType) - val subtypeNames0 = subtypeNames(typeCondName, rootType) - val isSubsetCondition = subtypeNames0.getOrElse(Set.empty) - loop( - f.selectionSet, - t, - fragment = Some(Fragment(Some(name), resolvedDirectives)), - targets = Some(Set(typeCondName)), - condition = subtypeNames0 - ).map { - case t @ (_, Some(c)) if isSubsetCondition(c) => t - case (f1, _) => (f1, Some(typeCondName)) - } + val f = fragments.getOrElse(name, null) + if ((f ne null) && checkDirectives(resolvedDirectives)) { + val typeCondName = f.typeCondition.name + val t = rootType.types.getOrElse(typeCondName, fieldType) + val subtypeNames0 = subtypeNames(typeCondName, rootType) + val isSubsetCondition = subtypeNames0.getOrElse(Set.empty) + loop( + f.selectionSet, + t, + fragment = Some(Fragment(Some(name), resolvedDirectives)), + targets = Some(Set(typeCondName)), + condition = subtypeNames0 + ) match { + case Left(l) => l.map((_, Some(typeCondName))) + case Right(l) => + l.map { + case t @ (_, Some(c)) if isSubsetCondition(c) => t + case (f1, _) => (f1, Some(typeCondName)) + } } - } else None - _fields.getOrElse(Nil) + } else Nil } ) - fields.foreach((addField _).tupled) + fields.foreach((builder.addField _).tupled) case InlineFragment(typeCondition, directives, selectionSet) => val resolvedDirectives = directives.map(resolveDirectiveVariables(variableValues, variableDefinitionsMap)) if (checkDirectives(resolvedDirectives)) { @@ -227,21 +226,26 @@ object Field { fragment = Some(Fragment(None, resolvedDirectives)), targets = typeName.map(Set(_)), condition = subtypeNames0 - ).foreach { case (f, c) => - if (c.exists(isSubsetCondition)) addField(f, c) - else addField(f, typeName) + ) match { + case Left(l) => l.foreach(builder.addField(_, typeName)) + case Right(l) => + l.foreach { + case (f, c) if c.exists(isSubsetCondition) => builder.addField(f, c) + case (f, _) => builder.addField(f, typeName) + } } } } - val builder = List.newBuilder[(Field, Option[String])] - map.forEach { case ((_, cond), field) => builder += ((field, cond)) } builder.result() } val fields = loop(selectionSet, fieldType, None, None, None) - Field("", fieldType, None, fields = fields.map(_._1), directives = directives) + Field("", fieldType, None, fields = fields.fold(identityFnList, _.map(_._1)), directives = directives) } + private val identityFnList: List[Field] => List[Field] = l => l + private val leftNil = Left(List.empty[Field]) + private def resolveDirectiveVariables( variableValues: Map[String, InputValue], variableDefinitions: Map[String, VariableDefinition] @@ -297,4 +301,34 @@ object Field { case Some(BooleanValue(value)) => value case _ => default } + + private abstract class FieldBuilder { + def addField(f: Field, condition: Option[String]): Unit + def result(): Either[List[Field], List[(Field, Option[String])]] + } + + private object FieldBuilder { + def forSelectionSet(selectionSet: List[Selection]): FieldBuilder = + if (selectionSet.forall(_.isInstanceOf[F])) new FieldsOnly else new Full + + private final class FieldsOnly extends FieldBuilder { + private[this] val builder = new ListBuffer[Field] + + def addField(f: Field, condition: Option[String]): Unit = builder += f + def result(): Either[List[Field], List[(Field, Option[String])]] = Left(builder.result()) + } + + private final class Full extends FieldBuilder { + private[this] val map = new java.util.LinkedHashMap[(String, Option[String]), Field]() + + def addField(f: Field, condition: Option[String]): Unit = + map.compute((f.aliasedName, condition), (_, existing) => if (existing eq null) f else existing.combine(f)) + + def result(): Either[List[Field], List[(Field, Option[String])]] = { + val builder = new ListBuffer[(Field, Option[String])] + map.forEach { case ((_, cond), field) => builder += ((field, cond)) } + Right(builder.result()) + } + } + } } From a5c180acb3d0c91bbd72c3ca502dbb65ed93e02c Mon Sep 17 00:00:00 2001 From: Kyri Petrou Date: Fri, 14 Jun 2024 19:37:56 +1000 Subject: [PATCH 2/2] Fold either directly after loop --- core/src/main/scala/caliban/execution/Field.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/caliban/execution/Field.scala b/core/src/main/scala/caliban/execution/Field.scala index dc6c9066e..c70753d1b 100644 --- a/core/src/main/scala/caliban/execution/Field.scala +++ b/core/src/main/scala/caliban/execution/Field.scala @@ -162,8 +162,9 @@ object Field { val t = if (selected eq null) Types.string else selected._type val fields = - if (selectionSet.nonEmpty) loop(selectionSet, t, None, None, None) - else leftNil // Fragments apply on to the direct children of the fragment spread + if (selectionSet.nonEmpty) + loop(selectionSet, t, None, None, None).fold(identityFnList, _.map(_._1)) + else Nil builder.addField( new Field( @@ -171,7 +172,7 @@ object Field { t, Some(innerType), alias, - fields.fold(identityFnList, _.map(_._1)), + fields, targets = targets, arguments = resolveVariables(arguments, variableDefinitionsMap, variableValues), directives = resolvedDirectives, @@ -244,7 +245,6 @@ object Field { } private val identityFnList: List[Field] => List[Field] = l => l - private val leftNil = Left(List.empty[Field]) private def resolveDirectiveVariables( variableValues: Map[String, InputValue],