Parser microoptimizations (#2246)

* Parser micro-optimizations * Use flatMap instead of positive-lookahead * Add back nowarn annotations * Revert change to `definition` * Remove blackhole usage from parser benchmarks
ghostdogpr · May 24, 2024 · c849e0e · c849e0e
1 parent 76e95b3
commit c849e0e
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 132 deletions.
diff --git a/benchmarks/src/main/scala/caliban/ComplexQueryBenchmark.scala b/benchmarks/src/main/scala/caliban/ComplexQueryBenchmark.scala
@@ -58,97 +58,94 @@ class ComplexQueryBenchmark {
 }
 
 object ComplexQueryBenchmark {
-  val fullIntrospectionQuery = """
-              query IntrospectionQuery {
-                __schema {
-                  queryType { name }
-                  mutationType { name }
-                  subscriptionType { name }
-                  types {
-                    ...FullType
-                  }
-                  directives {
-                    name
-                    description
-                    locations
-                    args {
-                      ...InputValue
-                    }
-                  }
-                }
-              }
-
-              fragment FullType on __Type {
-                kind
-                name
-                description
-                fields(includeDeprecated: true) {
-                  name
-                  description
-                  args {
-                    ...InputValue
-                  }
-                  type {
-                    ...TypeRef
-                  }
-                  isDeprecated
-                  deprecationReason
-                }
-                inputFields {
-                  ...InputValue
-                }
-                interfaces {
-                  ...TypeRef
-                }
-                enumValues(includeDeprecated: true) {
-                  name
-                  description
-                  isDeprecated
-                  deprecationReason
-                }
-                possibleTypes {
-                  ...TypeRef
-                }
-              }
-
-              fragment InputValue on __InputValue {
-                name
-                description
-                type { ...TypeRef }
-                defaultValue
-              }
-
-              fragment TypeRef on __Type {
-                kind
-                name
-                ofType {
-                  kind
-                  name
-                  ofType {
-                    kind
-                    name
-                    ofType {
-                      kind
-                      name
-                      ofType {
-                        kind
-                        name
-                        ofType {
-                          kind
-                          name
-                          ofType {
-                            kind
-                            name
-                            ofType {
-                              kind
-                              name
-                            }
-                          }
-                        }
-                      }
-                    }
-                  }
-                }
-              }
-                """
+  val fullIntrospectionQuery =
+    """query IntrospectionQuery {
+      |  __schema {
+      |    queryType { name }
+      |    mutationType { name }
+      |    subscriptionType { name }
+      |    types {
+      |      ...FullType
+      |    }
+      |    directives {
+      |      name
+      |      description
+      |      locations
+      |      args {
+      |        ...InputValue
+      |      }
+      |    }
+      |  }
+      |}
+      |fragment FullType on __Type {
+      |  kind
+      |  name
+      |  description
+      |  fields(includeDeprecated: true) {
+      |    name
+      |    description
+      |    args {
+      |      ...InputValue
+      |    }
+      |    type {
+      |      ...TypeRef
+      |    }
+      |    isDeprecated
+      |    deprecationReason
+      |  }
+      |  inputFields {
+      |    ...InputValue
+      |  }
+      |  interfaces {
+      |    ...TypeRef
+      |  }
+      |  enumValues(includeDeprecated: true) {
+      |    name
+      |    description
+      |    isDeprecated
+      |    deprecationReason
+      |  }
+      |  possibleTypes {
+      |    ...TypeRef
+      |  }
+      |}
+      |fragment InputValue on __InputValue {
+      |  name
+      |  description
+      |  type { ...TypeRef }
+      |  defaultValue
+      |}
+      |fragment TypeRef on __Type {
+      |  kind
+      |  name
+      |  ofType {
+      |    kind
+      |    name
+      |    ofType {
+      |      kind
+      |      name
+      |      ofType {
+      |        kind
+      |        name
+      |        ofType {
+      |          kind
+      |          name
+      |          ofType {
+      |            kind
+      |            name
+      |            ofType {
+      |              kind
+      |              name
+      |              ofType {
+      |                kind
+      |                name
+      |              }
+      |            }
+      |          }
+      |        }
+      |      }
+      |    }
+      |  }
+      |}
+      |""".stripMargin
 }
diff --git a/benchmarks/src/main/scala/caliban/ParserBenchmark.scala b/benchmarks/src/main/scala/caliban/ParserBenchmark.scala
@@ -1,51 +1,41 @@
 package caliban
 
 import caliban.parsing.Parser
-import cats.effect.IO
-import io.circe.Json
+import caliban.parsing.adt.Document
+import cats.data.NonEmptyList
+import cats.parse.Caret
+import gql.parser.QueryAst
+import grackle.Operation
 import org.openjdk.jmh.annotations._
-import sangria.execution._
-import sangria.marshalling.circe._
 import sangria.parser.QueryParser
 
 import java.util.concurrent.TimeUnit
-import scala.concurrent.duration._
-import scala.concurrent.{ Await, ExecutionContextExecutor, Future }
+import scala.concurrent.ExecutionContextExecutor
 
 @State(Scope.Thread)
 @BenchmarkMode(Array(Mode.Throughput))
 @OutputTimeUnit(TimeUnit.SECONDS)
-@Warmup(iterations = 5, time = 3, timeUnit = TimeUnit.SECONDS)
-@Measurement(iterations = 5, time = 3, timeUnit = TimeUnit.SECONDS)
+@Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
 @Fork(1)
 class ParserBenchmark {
   import ComplexQueryBenchmark._
 
   implicit val executionContext: ExecutionContextExecutor = scala.concurrent.ExecutionContext.global
 
   @Benchmark
-  def runCaliban(): Unit = {
-    val io = Parser.parseQuery(fullIntrospectionQuery)
-    Caliban.run(io)
-    ()
-  }
+  def runCaliban(): Document =
+    Parser.parseQueryEither(fullIntrospectionQuery).fold(throw _, identity)
 
   @Benchmark
-  def runSangria(): Unit = {
-    val future = Future.fromTry(QueryParser.parse(fullIntrospectionQuery))
-    Await.result(future, 1.minute)
-    ()
-  }
+  def runSangria(): sangria.ast.Document =
+    QueryParser.parse(fullIntrospectionQuery).fold(throw _, identity)
 
   @Benchmark
-  def runGrackle(): Unit = {
-    Grackle.compiler.compile(fullIntrospectionQuery)
-    ()
-  }
+  def runGrackle(): Operation =
+    Grackle.compiler.compile(fullIntrospectionQuery).getOrElse(throw new Throwable("Grackle failed to parse query"))
 
   @Benchmark
-  def runGql(): Unit = {
-    gql.parser.parseQuery(fullIntrospectionQuery)
-    ()
-  }
+  def runGql(): NonEmptyList[QueryAst.ExecutableDefinition[Caret]] =
+    gql.parser.parseQuery(fullIntrospectionQuery).fold(e => throw new Throwable(e.prettyError.value), identity)
 }
diff --git a/core/src/main/scala/caliban/parsing/parsers/Parsers.scala b/core/src/main/scala/caliban/parsing/parsers/Parsers.scala
@@ -15,7 +15,7 @@ import fastparse._
 
 import scala.annotation.nowarn
 
-@nowarn("msg=NoWhitespace") // False positive warning in Scala 2.x
+@nowarn("msg=NoWhitespace") // False positive warning in Scala 2.12.x
 object Parsers extends SelectionParsers {
   def argumentDefinition(implicit ev: P[Any]): P[InputValueDefinition]        =
     (stringValue.? ~ name ~ ":" ~ type_ ~ defaultValue.? ~ directives.?).map {

diff --git a/core/src/main/scala/caliban/parsing/parsers/SelectionParsers.scala b/core/src/main/scala/caliban/parsing/parsers/SelectionParsers.scala
@@ -8,11 +8,9 @@ import fastparse._
 
 import scala.annotation.nowarn
 
-@nowarn("msg=NoWhitespace") // False positive warning in Scala 2.x
+@nowarn("msg=NoWhitespace") // False positive warning in Scala 2.12.x
 private[caliban] trait SelectionParsers extends ValueParsers {
 
-  @deprecated("Kept for bincompat only, scheduled to be removed")
-  def alias(implicit ev: P[Any]): P[String]       = name ~ ":"
   def aliasOrName(implicit ev: P[Any]): P[String] = ":" ~/ name
 
   def argument(implicit ev: P[Any]): P[(String, InputValue)]     = name ~ ":" ~ value
@@ -30,19 +28,13 @@ private[caliban] trait SelectionParsers extends ValueParsers {
   def namedType(implicit ev: P[Any]): P[NamedType] = name.filter(_ != "null").map(NamedType(_, nonNull = false))
   def listType(implicit ev: P[Any]): P[ListType]   = ("[" ~ type_ ~ "]").map(t => ListType(t, nonNull = false))
 
-  @deprecated("Kept for bincompat only, scheduled to be removed")
-  def nonNullType(implicit ev: P[Any]): P[Type] = ((namedType | listType) ~ "!").map {
-    case t: NamedType => t.copy(nonNull = true)
-    case t: ListType  => t.copy(nonNull = true)
-  }
-
   def type_(implicit ev: P[Any]): P[Type] = ((namedType | listType) ~ "!".!.?).map {
     case (t: NamedType, nn) => if (nn.isDefined) t.copy(nonNull = true) else t
     case (t: ListType, nn)  => if (nn.isDefined) t.copy(nonNull = true) else t
   }
 
   def field(implicit ev: P[Any]): P[Field] =
-    (Index ~ name ~ aliasOrName.? ~ arguments.? ~ directives.? ~ selectionSet.?).map {
+    (Index ~~ name ~ aliasOrName.? ~ arguments.? ~ directives.? ~ selectionSet.?).map {
       case (index, alias, Some(name), args, dirs, sels) =>
         Field(Some(alias), name, args.getOrElse(Map()), dirs.getOrElse(Nil), sels.getOrElse(Nil), index)
       case (index, name, _, args, dirs, sels)           =>

diff --git a/core/src/main/scala/caliban/parsing/parsers/StringParsers.scala b/core/src/main/scala/caliban/parsing/parsers/StringParsers.scala
@@ -57,7 +57,12 @@ private[caliban] trait StringParsers {
 
   def sourceCharacter(implicit ev: P[Any]): P[Unit]                      = CharIn("\u0009\u000A\u000D\u0020-\uFFFF")
   def sourceCharacterWithoutLineTerminator(implicit ev: P[Any]): P[Unit] = CharIn("\u0009\u0020-\uFFFF")
-  def name(implicit ev: P[Any]): P[String]                               = (CharIn("_A-Za-z") ~~ CharIn("_0-9A-Za-z").repX).!
+  def name(implicit ev: P[Any]): P[String]                               =
+    CharsWhileIn("_0-9A-Za-z", 1).!.flatMap { s =>
+      // Less efficient in case of an error, but more efficient in case of success (happy path)
+      if (s.charAt(0) <= '9') ev.freshFailure()
+      else ev.freshSuccess(s)
+    }
   def nameOnly(implicit ev: P[Any]): P[String]                           = Start ~ name ~ End
 
   def hexDigit(implicit ev: P[Any]): P[Unit]         = CharIn("0-9a-fA-F")

diff --git a/core/src/main/scala/caliban/parsing/parsers/ValueParsers.scala b/core/src/main/scala/caliban/parsing/parsers/ValueParsers.scala
@@ -7,7 +7,7 @@ import fastparse._
 
 import scala.annotation.nowarn
 
-@nowarn("msg=NoWhitespace") // False positive warning in Scala 2.x
+@nowarn("msg=NoWhitespace") // False positive warning in Scala 2.12.x
 private[caliban] trait ValueParsers extends NumberParsers {
   def booleanValue(implicit ev: P[Any]): P[BooleanValue] =
     StringIn("true", "false").!.map(v => BooleanValue(v.toBoolean))