Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize fragment validation #2039

Merged
merged 3 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -669,22 +669,13 @@ lazy val commonSettings = Def.settings(
})
)

lazy val enforceMimaCompatibility = true // Enable / disable failing CI on binary incompatibilities
lazy val enforceMimaCompatibility = false // Enable / disable failing CI on binary incompatibilities
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS: I disabled this since I had to add too many filters due to the removed methods


lazy val enableMimaSettingsJVM =
Def.settings(
mimaFailOnProblem := enforceMimaCompatibility,
mimaPreviousArtifacts := previousStableVersion.value.map(organization.value %% moduleName.value % _).toSet,
mimaBinaryIssueFilters ++= Seq(
ProblemFilters.exclude[IncompatibleMethTypeProblem]("caliban.schema.Step#ObjectStep*"),
ProblemFilters.exclude[IncompatibleResultTypeProblem]("caliban.schema.Step#ObjectStep*"),
ProblemFilters.exclude[DirectMissingMethodProblem]("caliban.schema.Annotations*"),
ProblemFilters.exclude[MissingTypesProblem]("caliban.schema.Annotations*"),
ProblemFilters.exclude[IncompatibleResultTypeProblem]("caliban.schema.Annotations*"),
ProblemFilters.exclude[IncompatibleMethTypeProblem]("caliban.Quick*"),
ProblemFilters.exclude[DirectMissingMethodProblem]("caliban.Quick*"),
ProblemFilters.exclude[DirectMissingMethodProblem]("caliban.quick.package*")
)
mimaBinaryIssueFilters ++= Seq()
)

lazy val enableMimaSettingsJS =
Expand Down
32 changes: 19 additions & 13 deletions core/src/main/scala/caliban/execution/Executor.scala
Original file line number Diff line number Diff line change
Expand Up @@ -64,20 +64,26 @@ object Executor {
): ReducedStep[R] = {

def reduceObjectStep(objectName: String, getFieldStep: String => Step[R]): ReducedStep[R] = {
def reduceField(f: Field): (String, ReducedStep[R], FieldInfo) = {
val field =
if (f.name == "__typename") PureStep(StringValue(objectName))
else reduceStep(getFieldStep(f.name), f, f.arguments, Left(f.aliasedName) :: path)
(f.aliasedName, field, fieldInfo(f, path, f.directives))
}

val filteredFields = mergeFields(currentField, objectName)
val (deferred, eager) = filteredFields.partitionMap {
case f @ Field("__typename", _, _, _, _, _, _, directives, _, _, _) =>
Right((f.aliasedName, PureStep(StringValue(objectName)), fieldInfo(f, path, directives)))
case f @ Field(name, _, _, _, _, _, args, directives, _, _, fragment) =>
val field = reduceStep(getFieldStep(name), f, args, Left(f.aliasedName) :: path)
val entry = (f.aliasedName, field, fieldInfo(f, path, directives))

fragment match {
// The defer spec provides some latitude on how we handle responses. Since it is more performant to return
// pure fields rather than spin up the defer machinery we return pure fields immediately to the caller.
case Some(IsDeferred(label)) if isDeferredEnabled && !field.isPure => Left((label, entry))
case _ => Right(entry)
val (deferred, eager) = {
if (isDeferredEnabled) {
filteredFields.partitionMap { f =>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That one's pretty good! I ran the profiler a whole ago and I remember this partitionMap being quite visible in the flamegraph.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah besides avoiding wrapping the steps into an Either, .map is about twice as fast as manually creating a list with a ListBuilder (which is what partitionMap uses)

val entry = reduceField(f)
f.fragment match {
// The defer spec provides some latitude on how we handle responses. Since it is more performant to return
// pure fields rather than spin up the defer machinery we return pure fields immediately to the caller.
case Some(IsDeferred(label)) if !entry._2.isPure => Left((label, entry))
case _ => Right(entry)
}
}
} else (Nil, filteredFields.map(reduceField))
}

val eagerReduced = reduceObject(eager, wrapPureValues)
Expand Down Expand Up @@ -303,7 +309,7 @@ object Executor {

for {
cache <- Cache.empty
reduced = reduceStep(plan, request.field, Map(), Nil)
reduced = reduceStep(plan, request.field, Map.empty, Nil)
response <- runQuery(reduced, cache)
} yield response
}
Expand Down
69 changes: 34 additions & 35 deletions core/src/main/scala/caliban/execution/Field.scala
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,13 @@ object Field {
val memoizedFragments = new mutable.HashMap[String, (List[Field], Option[String])]()
val variableDefinitionsMap = variableDefinitions.map(v => v.name -> v).toMap

def loop(selectionSet: List[Selection], fieldType: __Type, fragment: Option[Fragment]): List[Field] = {
def loop(
selectionSet: List[Selection],
fieldType: __Type,
fragment: Option[Fragment],
targets: Option[Set[String]],
condition: Option[Set[String]]
): List[Field] = {
val map = new java.util.LinkedHashMap[(String, Option[String]), Field]()

def addField(f: Field, condition: Option[String]): Unit =
Expand All @@ -141,22 +147,22 @@ object Field {
val t = selected.fold(Types.string)(_._type) // default only case where it's not found is __typename

val fields =
if (selectionSet.nonEmpty) loop(selectionSet, t, None)
if (selectionSet.nonEmpty) loop(selectionSet, t, None, None, None)
else Nil // Fragments apply on to the direct children of the fragment spread

addField(
Field(
new Field(
name,
t,
Some(innerType),
alias,
fields,
None,
resolveVariables(arguments, variableDefinitionsMap, variableValues),
resolvedDirectives,
None,
() => sourceMapper.getLocation(index),
fragment
targets = targets,
arguments = resolveVariables(arguments, variableDefinitionsMap, variableValues),
directives = resolvedDirectives,
_condition = condition,
_locationInfo = () => sourceMapper.getLocation(index),
fragment = fragment
),
None
)
Expand All @@ -165,17 +171,16 @@ object Field {
val (fields, condition) = memoizedFragments.getOrElseUpdate(
name, {
val resolvedDirectives = directives.map(resolveDirectiveVariables(variableValues, variableDefinitionsMap))

val _fields = if (checkDirectives(resolvedDirectives)) {
val _fields = if (checkDirectives(resolvedDirectives)) {
fragments.get(name).map { f =>
val t = rootType.types.getOrElse(f.typeCondition.name, fieldType)
val _targets = Some(Set(f.typeCondition.name))
val _condition = subtypeNames(f.typeCondition.name, rootType)

loop(f.selectionSet, t, Some(Fragment(Some(name), resolvedDirectives))).map { field =>
if (field._condition.isDefined) field
else field.copy(targets = _targets, _condition = _condition)
} -> Some(f.typeCondition.name)
val t = rootType.types.getOrElse(f.typeCondition.name, fieldType)
loop(
f.selectionSet,
t,
fragment = Some(Fragment(Some(name), resolvedDirectives)),
targets = Some(Set(f.typeCondition.name)),
condition = subtypeNames(f.typeCondition.name, rootType)
) -> Some(f.typeCondition.name)
}
} else None
_fields.getOrElse(Nil -> None)
Expand All @@ -184,31 +189,25 @@ object Field {
fields.foreach(addField(_, condition))
case InlineFragment(typeCondition, directives, selectionSet) =>
val resolvedDirectives = directives.map(resolveDirectiveVariables(variableValues, variableDefinitionsMap))

if (checkDirectives(resolvedDirectives)) {
val t = innerType.possibleTypes
val t = innerType.possibleTypes
.flatMap(_.find(_.name.exists(typeCondition.map(_.name).contains)))
.orElse(typeCondition.flatMap(typeName => rootType.types.get(typeName.name)))
.getOrElse(fieldType)
val fields = loop(selectionSet, t, Some(Fragment(None, resolvedDirectives)))
typeCondition match {
case None => fields.map(addField(_, None))
case Some(typeName) =>
val _targets = Some(Set(typeName.name))
val _condition = subtypeNames(typeName.name, rootType)
fields.foreach { field =>
val _field =
if (field._condition.isDefined) field
else field.copy(targets = _targets, _condition = _condition)
addField(_field, Some(typeName.name))
}
}
val typeName = typeCondition.map(_.name)
loop(
selectionSet,
t,
fragment = Some(Fragment(None, resolvedDirectives)),
targets = typeName.map(Set(_)),
condition = typeName.flatMap(subtypeNames(_, rootType))
).foreach(addField(_, typeName))
}
}
map.values().asScala.toList
}

val fields = loop(selectionSet, fieldType, None)
val fields = loop(selectionSet, fieldType, None, None, None)
Field("", fieldType, None, fields = fields, directives = directives)
}

Expand Down
2 changes: 2 additions & 0 deletions core/src/main/scala/caliban/introspection/adt/__Field.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ case class __Field(
deprecationReason: Option[String] = None,
@GQLExcluded directives: Option[List[Directive]] = None
) {
final override lazy val hashCode: Int = super.hashCode()

def toFieldDefinition: FieldDefinition = {
val allDirectives = (if (isDeprecated)
List(
Expand Down
16 changes: 14 additions & 2 deletions core/src/main/scala/caliban/introspection/adt/__Type.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ case class __Type(
@GQLExcluded directives: Option[List[Directive]] = None,
@GQLExcluded origin: Option[String] = None
) { self =>
final override lazy val hashCode: Int = super.hashCode()

def |+|(that: __Type): __Type = __Type(
kind,
(name ++ that.name).reduceOption((_, b) => b),
Expand Down Expand Up @@ -130,8 +132,18 @@ case class __Type(
lazy val allEnumValues: List[__EnumValue] =
enumValues(__DeprecatedArgs(Some(true))).getOrElse(Nil)

private[caliban] lazy val allFieldsMap: Map[String, __Field] =
allFields.map(f => f.name -> f).toMap
private[caliban] lazy val allFieldsMap: collection.Map[String, __Field] = {
val map = collection.mutable.HashMap.empty[String, __Field]
allFields.foreach(f => map.update(f.name, f))
map
}

lazy val innerType: __Type = Types.innerType(this)

private[caliban] lazy val possibleTypeNames: Set[String] =
kind match {
case __TypeKind.OBJECT => name.fold(Set.empty[String])(Set(_))
case __TypeKind.INTERFACE | __TypeKind.UNION => possibleTypes.fold(Set.empty[String])(_.flatMap(_.name).toSet)
case _ => Set.empty
}
}
4 changes: 3 additions & 1 deletion core/src/main/scala/caliban/parsing/adt/Selection.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package caliban.parsing.adt
import caliban.InputValue
import caliban.parsing.adt.Type.NamedType

sealed trait Selection
sealed trait Selection {
final override lazy val hashCode: Int = super.hashCode()
}

object Selection {

Expand Down
26 changes: 10 additions & 16 deletions core/src/main/scala/caliban/validation/FragmentValidator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import caliban.CalibanError.ValidationError
import caliban.introspection.adt._
import caliban.parsing.adt.Selection
import caliban.validation.Utils._
import caliban.validation.Utils.syntax._
import zio.Chunk
import zio.prelude.EReader
import zio.prelude._
import zio.prelude.fx.ZPure

import scala.collection.mutable
import scala.util.hashing.MurmurHash3

object FragmentValidator {
def findConflictsWithinSelectionSet(
Expand All @@ -24,13 +24,13 @@ object FragmentValidator {
val groupsCache = mutable.Map.empty[Int, Chunk[Set[SelectedField]]]

def sameResponseShapeByName(set: Iterable[Selection]): Chunk[String] = {
val keyHash = set.hashCode()
val keyHash = MurmurHash3.unorderedHash(set)
shapeCache.get(keyHash) match {
case Some(value) => value
case None =>
val fields = FieldMap(context, parentType, set)
val res = Chunk.fromIterable(fields.flatMap { case (name, values) =>
cross(values).flatMap { case (f1, f2) =>
cross(values, includeIdentity = true).flatMap { case (f1, f2) =>
if (doTypesConflict(f1.fieldDef._type, f2.fieldDef._type)) {
Chunk(
s"$name has conflicting types: ${f1.parentType.name.getOrElse("")}.${f1.fieldDef.name} and ${f2.parentType.name
Expand All @@ -46,7 +46,7 @@ object FragmentValidator {
}

def sameForCommonParentsByName(set: Iterable[Selection]): Chunk[String] = {
val keyHash = set.hashCode()
val keyHash = MurmurHash3.unorderedHash(set)
parentsCache.get(keyHash) match {
case Some(value) => value
case None =>
Expand Down Expand Up @@ -81,14 +81,14 @@ object FragmentValidator {
false

def requireSameNameAndArguments(fields: Set[SelectedField]) =
cross(fields).flatMap { case (f1, f2) =>
cross(fields, includeIdentity = false).flatMap { case (f1, f2) =>
if (f1.fieldDef.name != f2.fieldDef.name) {
List(
Some(
s"${f1.parentType.name.getOrElse("")}.${f1.fieldDef.name} and ${f2.parentType.name.getOrElse("")}.${f2.fieldDef.name} are different fields."
)
} else if (f1.selection.arguments != f2.selection.arguments)
List(s"${f1.fieldDef.name} and ${f2.fieldDef.name} have different arguments")
else List()
Some(s"${f1.fieldDef.name} and ${f2.fieldDef.name} have different arguments")
else None
}

def groupByCommonParents(fields: Set[SelectedField]): Chunk[Set[SelectedField]] = {
Expand All @@ -109,13 +109,7 @@ object FragmentValidator {
_,
_
) if isConcrete(field.parentType) =>
concreteGroups.get(name) match {
case Some(v) => v += field
case None =>
val sb = Set.newBuilder ++= abstractGroup
sb += field
concreteGroups.update(name, sb)
}
concreteGroups.getOrElseUpdate(name, Set.newBuilder ++= abstractGroup) += field
case _ => ()
}

Expand Down
37 changes: 21 additions & 16 deletions core/src/main/scala/caliban/validation/Utils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import caliban.introspection.adt.__TypeKind._
import caliban.parsing.adt.Type.NamedType
import zio.Chunk

import scala.collection.compat._

object Utils {
def isObjectType(t: __Type): Boolean =
t.kind match {
Expand Down Expand Up @@ -56,22 +58,25 @@ object Utils {
def getType(t: NamedType, context: Context): Option[__Type] =
context.rootType.types.get(t.name)

def cross[A](a: Iterable[A]): Chunk[(A, A)] =
Chunk.fromIterable(for (xs <- a; ys <- a) yield (xs, ys))

def cross[A](a: Iterable[A], b: Iterable[A]): Chunk[(A, A)] =
Chunk.fromIterable(for (xs <- a; ys <- b) yield (xs, ys))

object syntax {
implicit class OptionSyntax[+A](val self: Option[A]) extends AnyVal {
def zip[B](that: Option[B]): Option[(A, B)] =
self.flatMap(a => that.map(b => (a, b)))
/**
* For an iterable, produce a Chunk containing tuples of all possible unique combinations, optionally including the identity
*/
def cross[A](a: Iterable[A], includeIdentity: Boolean): Chunk[(A, A)] = {
val ca = Chunk.fromIterable(a)
val size = ca.size
val cb = Chunk.newBuilder[(A, A)]
var i1, i2 = 0
val modifier = if (includeIdentity) 0 else 1
while (i1 < size - modifier) {
i2 = i1 + modifier
val l = ca(i1)
while (i2 < size) {
cb += ((l, ca(i2)))
i2 += 1
}
i1 += 1
}

implicit class Tuple2Syntax[+A, +B](val self: (Option[A], Option[B])) extends AnyVal {
def mapN[C](f: (A, B) => C): Option[C] =
self._1.flatMap(a => self._2.map(b => f(a, b)))
}

cb.result()
}

}
Loading