Merge pull request #246 from gemini-hlsw/topic/fewer-lists

Reduce use of Lists and miscellaneous performance tweaks

build.sbt

@@ -26,7 +26,7 @@ val Scala3 = "3.1.3"
 ThisBuild / scalaVersion := Scala2
 ThisBuild / crossScalaVersions := Seq(Scala2, Scala3)
 
-ThisBuild / tlBaseVersion := "0.3"
+ThisBuild / tlBaseVersion := "0.4"
 ThisBuild / organization := "edu.gemini"
 ThisBuild / organizationName := "Association of Universities for Research in Astronomy, Inc. (AURA)"
 ThisBuild / startYear := Some(2019)

modules/circe/src/main/scala/circemapping.scala

@@ -4,6 +4,8 @@
 package edu.gemini.grackle
 package circe
 
+import scala.collection.Factory
+
 import cats.Monad
 import cats.implicits._
 import fs2.Stream
@@ -78,9 +80,9 @@ abstract class CirceMapping[F[_]: Monad] extends Mapping[F] {
 
  def isList: Boolean = tpe.isList && focus.isArray
 
- def asList: Result[List[Cursor]] = tpe match {
+ def asList[C](factory: Factory[Cursor, C]): Result[C] = tpe match {
  case ListType(elemTpe) if focus.isArray =>
- focus.asArray.map(_.map(e => mkChild(context.asType(elemTpe), e)).toList)
+ focus.asArray.map(_.view.map(e => mkChild(context.asType(elemTpe), e)).to(factory))
  .toRightIor(mkOneError(s"Expected List type, found $tpe for focus ${focus.noSpaces}"))
  case _ =>
  mkErrorResult(s"Expected List type, found $tpe for focus ${focus.noSpaces}")

modules/core/src/main/scala/cursor.scala

@@ -3,6 +3,7 @@
 
 package edu.gemini.grackle
 
+import scala.collection.Factory
 import scala.reflect.{classTag, ClassTag}
 
 import cats.data.Ior
@@ -61,7 +62,14 @@ trait Cursor {
  * this `Cursor` if it is of a list type, or an error or the left hand side
  * otherwise.
  */
- def asList: Result[List[Cursor]]
+ final def asList: Result[List[Cursor]] = asList(List)
+
+ /**
+ * Yield a collection of `Cursor`s corresponding to the elements of the value at
+ * this `Cursor` if it is of a list type, or an error or the left hand side
+ * otherwise.
+ */
+ def asList[C](factory: Factory[Cursor, C]): Result[C]
 
  /** Is the value at this `Cursor` of a nullable type? */
  def isNullable: Boolean
@@ -231,28 +239,27 @@ object Cursor {
  */
  case class Context(
  rootTpe: Type,
- path0: List[(String, String, Type)] = Nil
+ path: List[String],
+ resultPath: List[String],
+ typePath: List[Type]
  ) {
- lazy val path: List[String] = path0.map(_._1)
- lazy val resultPath: List[String] = path0.map(_._2)
- lazy val typePath = path0.map(_._3)
- lazy val tpe: Type = path0.headOption.map(_._3).getOrElse(rootTpe)
+ lazy val tpe: Type = typePath.headOption.getOrElse(rootTpe)
 
  def asType(tpe: Type): Context = {
- path0 match {
+ typePath match {
  case Nil => copy(rootTpe = tpe)
- case hd :: tl => copy(path0 = (hd._1, hd._2, tpe) :: tl)
+ case _ :: tl => copy(typePath = tpe :: tl)
  }
  }
 
  def forField(fieldName: String, resultName: String): Option[Context] =
  tpe.underlyingField(fieldName).map { fieldTpe =>
- copy(path0 = (fieldName, resultName, fieldTpe) :: path0)
+ copy(path = fieldName :: path, resultPath = resultName :: resultPath, typePath = fieldTpe :: typePath)
  }
 
  def forField(fieldName: String, resultName: Option[String]): Option[Context] =
  tpe.underlyingField(fieldName).map { fieldTpe =>
- copy(path0 = (fieldName, resultName.getOrElse(fieldName), fieldTpe) :: path0)
+ copy(path = fieldName :: path, resultPath = resultName.getOrElse(fieldName) :: resultPath, typePath = fieldTpe :: typePath)
  }
 
  def forPath(path1: List[String]): Option[Context] =
@@ -263,13 +270,13 @@ object Cursor {
 
  def forFieldOrAttribute(fieldName: String, resultName: Option[String]): Context = {
  val fieldTpe = tpe.underlyingField(fieldName).getOrElse(ScalarType.AttributeType)
- copy(path0 = (fieldName, resultName.getOrElse(fieldName), fieldTpe) :: path0)
+ copy(path = fieldName :: path, resultPath = resultName.getOrElse(fieldName) :: resultPath, typePath = fieldTpe :: typePath)
  }
 
  override def equals(other: Any): Boolean =
  other match {
- case Context(oRootTpe, oPath0) =>
- rootTpe =:= oRootTpe && path0.corresponds(oPath0)((x, y) => x._1 == y._1 && x._2 == y._2)
+ case Context(oRootTpe, oPath, oResultPath, _) =>
+ rootTpe =:= oRootTpe && resultPath == oResultPath && path == oPath
  case _ => false
  }
 
@@ -280,10 +287,10 @@ object Cursor {
  def apply(rootTpe: Type, fieldName: String, resultName: Option[String]): Option[Context] = {
  for {
  tpe <- rootTpe.underlyingField(fieldName)
- } yield new Context(rootTpe, List((fieldName, resultName.getOrElse(fieldName), tpe)))
+ } yield new Context(rootTpe, List(fieldName), List(resultName.getOrElse(fieldName)), List(tpe))
  }
 
- def apply(rootTpe: Type): Context = Context(rootTpe, Nil)
+ def apply(rootTpe: Type): Context = Context(rootTpe, Nil, Nil, Nil)
  }
 
  def flatten(c: Cursor): Result[List[Cursor]] =

modules/core/src/main/scala/query.scala

@@ -139,7 +139,7 @@ object Query {
  }
 
  case class OrderSelections(selections: List[OrderSelection[_]]) {
- def order(lc: List[Cursor]): List[Cursor] = {
+ def order(lc: Seq[Cursor]): Seq[Cursor] = {
  def cmp(x: Cursor, y: Cursor): Int = {
  @tailrec
  def loop(sels: List[OrderSelection[_]]): Int =

modules/core/src/main/scala/queryinterpreter.scala

@@ -8,7 +8,7 @@ import scala.collection.mutable
 import scala.jdk.CollectionConverters._
 
 import cats.Monoid
-import cats.data.{ Chain, Ior, IorT, Kleisli, NonEmptyChain }
+import cats.data.{ Chain, Ior, IorT, NonEmptyChain }
 import cats.implicits._
 import fs2.Stream
 import io.circe.Json
@@ -235,10 +235,10 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) {
  c0.asNullable.flatMap {
  case None => 0.rightIor
  case Some(c1) =>
- if (c1.isList) c1.asList.map(c2 => c2.size)
+ if (c1.isList) c1.asList(Iterator).map(_.size)
  else 1.rightIor
  }
- else if (c0.isList) c0.asList.map(c2 => c2.size)
+ else if (c0.isList) c0.asList(Iterator).map(_.size)
  else 1.rightIor
  }.map { value => List((fieldName, ProtoJson.fromJson(Json.fromInt(value)))) }
 
@@ -253,29 +253,61 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) {
  }
  }
 
- def runList(query: Query, tpe: Type, cursors: List[Cursor], f: Kleisli[Result, List[Cursor], List[Cursor]]): Result[ProtoJson] =
- if (cursors.exists(cursor => !cursorCompatible(tpe, cursor.tpe)))
- mkErrorResult(s"Mismatched query and cursor type in runList: $tpe ${cursors.map(_.tpe)}")
- else {
+ def runList(query: Query, tpe: Type, cursors: Iterator[Cursor], unique: Boolean, nullable: Boolean): Result[ProtoJson] = {
+ val (child, ic) =
  query match {
- case Filter(pred, child) =>
- runList(child, tpe, cursors, f.compose(_.filterA(pred(_))))
-
- case Limit(num, child) =>
- runList(child, tpe, cursors, f.compose(_.take(num).rightIor))
+ case FilterOrderByOffsetLimit(pred, selections, offset, limit, child) =>
+ val filtered =
+ pred.map { p =>
+ cursors.filter { c =>
+ p(c) match {
+ case left@Ior.Left(_) => return left
+ case Ior.Right(c) => c
+ case Ior.Both(_, c) => c
+ }
+ }
+ }.getOrElse(cursors)
+ val sorted = selections.map(OrderSelections(_).order(filtered.toSeq).iterator).getOrElse(filtered)
+ val sliced = (offset, limit) match {
+ case (None, None) => sorted
+ case (Some(off), None) => sorted.drop(off)
+ case (None, Some(lim)) => sorted.take(lim)
+ case (Some(off), Some(lim)) => sorted.slice(off, off+lim)
+ }
+ (child, sliced)
+ case other => (other, cursors)
+ }
 
- case Offset(num, child) =>
- runList(child, tpe, cursors, f.compose(_.drop(num).rightIor))
+ val builder = Vector.newBuilder[ProtoJson]
+ var problems = Chain.empty[Problem]
+ builder.sizeHint(ic.knownSize)
+ while(ic.hasNext) {
+ val c = ic.next()
+ if (!cursorCompatible(tpe, c.tpe))
+ return mkErrorResult(s"Mismatched query and cursor type in runList: $tpe ${cursors.map(_.tpe)}")
+
+ runValue(child, tpe, c) match {
+ case left@Ior.Left(_) => return left
+ case Ior.Right(v) => builder.addOne(v)
+ case Ior.Both(ps, v) =>
+ builder.addOne(v)
+ problems = problems.concat(ps.toChain)
+ }
+ }
 
-  case OrderBy(selections, child) =>
-  runList(child, tpe, cursors, f.compose(selections.order(_).rightIor))
+ def mkResult(j: ProtoJson): Result[ProtoJson] =
+ NonEmptyChain.fromChain(problems).map(neps => Ior.Both(neps, j)).getOrElse(j.rightIor)
 
- case _ =>
- f.run(cursors).flatMap(lc =>
- lc.traverse(c => runValue(query, tpe, c)).map(ProtoJson.fromValues)
- )
- }
+ if (!unique) mkResult(ProtoJson.fromValues(builder.result()))
+ else {
+ val size = builder.knownSize
+ if (size == 1) mkResult(builder.result()(0))
+ else if (size == 0) {
+ if(nullable) mkResult(ProtoJson.fromJson(Json.Null))
+ else mkErrorResult(s"No match")
+ } else mkErrorResult(s"Multiple matches")
  }
+ }
 
  /**
  * Interpret `query` against `cursor` with expected type `tpe`.
@@ -298,9 +330,18 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) {
 
  case (Wrap(_, Component(_, _, _)), ListType(tpe)) =>
  // Keep the wrapper with the component when going under the list
- cursor.asList.flatMap(lc =>
- lc.traverse(c => runValue(query, tpe, c)).map(ProtoJson.fromValues)
- )
+ cursor.asList(Iterator).map { ic =>
+ val builder = Vector.newBuilder[ProtoJson]
+ builder.sizeHint(ic.knownSize)
+ while(ic.hasNext) {
+ val c = ic.next()
+ runValue(query, tpe, c) match {
+ case Ior.Right(v) => builder.addOne(v)
+ case left => return left
+ }
+ }
+ ProtoJson.fromValues(builder.result())
+ }
 
  case (Wrap(_, Defer(_, _, _)), _) if cursor.isNull =>
  ProtoJson.fromJson(Json.Null).rightIor
@@ -346,46 +387,23 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) {
  }
  else stage(cursor)
 
- case (Unique(Filter(pred, child)), _) =>
- val cursors =
- if (cursor.isNullable)
- cursor.asNullable.flatMap {
- case None => Nil.rightIor
- case Some(c) => c.asList
- }
- else cursor.asList
-
- cursors.flatMap(_.filterA(pred(_))).flatMap(lc =>
- lc match {
- case List(c) => runValue(child, tpe.nonNull, c)
- case Nil if tpe.isNullable => ProtoJson.fromJson(Json.Null).rightIor
- case Nil => mkErrorResult(s"No match")
- case _ => mkErrorResult(s"Multiple matches")
- }
- )
-
  case (Unique(child), _) =>
  val oc =
  if (cursor.isNullable) cursor.asNullable
  else Some(cursor).rightIor
 
  oc.flatMap {
  case Some(c) =>
- runValue(child, tpe.nonNull.list, c).flatMap { pj =>
- ProtoJson.unpackList(pj).map {
- case Nil if tpe.isNullable => ProtoJson.fromJson(Json.Null).rightIor
- case Nil => mkErrorResult(s"No match")
- case List(elem) => elem.rightIor
- case _ => mkErrorResult(s"Multiple matches")
- }.getOrElse(mkErrorResult(s"Unique result of the wrong shape: $pj"))
+ c.asList(Iterator).flatMap { cursors =>
+ runList(child, tpe.nonNull, cursors, true, tpe.isNullable)
  }
  case None =>
  ProtoJson.fromJson(Json.Null).rightIor
  }
 
  case (_, ListType(tpe)) =>
- cursor.asList.flatMap { cursors =>
- runList(query, tpe, cursors, Kleisli(_.rightIor))
+ cursor.asList(Iterator).flatMap { cursors =>
+ runList(query, tpe, cursors, false, false)
  }
 
  case (_, NullableType(tpe)) =>
@@ -423,9 +441,9 @@ object QueryInterpreter {
  // A result which is deferred to the next stage or component of this interpreter.
  private[QueryInterpreter] case class StagedJson[F[_]](interpreter: QueryInterpreter[F], query: Query, rootTpe: Type, env: Env) extends DeferredJson
  // A partially constructed object which has at least one deferred subtree.
- private[QueryInterpreter] case class ProtoObject(fields: List[(String, ProtoJson)])
+ private[QueryInterpreter] case class ProtoObject(fields: Seq[(String, ProtoJson)])
  // A partially constructed array which has at least one deferred element.
- private[QueryInterpreter] case class ProtoArray(elems: List[ProtoJson])
+ private[QueryInterpreter] case class ProtoArray(elems: Seq[ProtoJson])
  // A result which will yield a selection from its child
  private[QueryInterpreter] case class ProtoSelect(elem: ProtoJson, fieldName: String)
 
@@ -444,9 +462,9 @@ object QueryInterpreter {
  * If all fields are complete then they will be combined as a complete
  * Json object.
  */
- def fromFields(fields: List[(String, ProtoJson)]): ProtoJson =
+ def fromFields(fields: Seq[(String, ProtoJson)]): ProtoJson =
  if(fields.forall(_._2.isInstanceOf[Json]))
- wrap(Json.fromFields(fields.asInstanceOf[List[(String, Json)]]))
+ wrap(Json.fromFields(fields.asInstanceOf[Seq[(String, Json)]]))
  else
  wrap(ProtoObject(fields))
 
@@ -456,9 +474,9 @@ object QueryInterpreter {
  * If all values are complete then they will be combined as a complete
  * Json array.
  */
- def fromValues(elems: List[ProtoJson]): ProtoJson =
+ def fromValues(elems: Seq[ProtoJson]): ProtoJson =
  if(elems.forall(_.isInstanceOf[Json]))
- wrap(Json.fromValues(elems.asInstanceOf[List[Json]]))
+ wrap(Json.fromValues(elems.asInstanceOf[Seq[Json]]))
  else
  wrap(ProtoArray(elems))
 
@@ -508,29 +526,7 @@ object QueryInterpreter {
  }
  }
 
- def unpackObject(p: ProtoJson): Option[List[ProtoJson]] =
- p match {
- case ProtoObject(List((_, packedElems))) => unpackList(packedElems)
- case j: Json if j.isObject =>
- j.asObject.flatMap(jo =>
- if (jo.size != 1) None
- else {
- val List((_, packedElems)) = jo.toList
- packedElems.asArray.map(v => wrapList(v.toList))
- }
- )
- case _ => None
- }
-
- def unpackList(p: ProtoJson): Option[List[ProtoJson]] =
- p match {
- case ProtoArray(elems) => Some(elems)
- case j: Json if j.isArray => j.asArray.map(ja => wrapList(ja.toList))
- case _ => None
- }
-
  private def wrap(j: AnyRef): ProtoJson = j.asInstanceOf[ProtoJson]
- private def wrapList(l: List[AnyRef]): List[ProtoJson] = l.asInstanceOf[List[ProtoJson]]
  }
 
  import ProtoJson._
@@ -600,7 +596,7 @@ object QueryInterpreter {
  case p: Json => p
  case d: DeferredJson => subst(d)
  case ProtoObject(fields) =>
- val newFields: List[(String, Json)] =
+ val newFields: Seq[(String, Json)] =
  fields.flatMap { case (label, pvalue) =>
  val value = loop(pvalue)
  if (isDeferred(pvalue) && value.isObject) {

modules/core/src/main/scala/valuemapping.scala

@@ -3,6 +3,7 @@
 
 package edu.gemini.grackle
 
+import scala.collection.Factory
 import scala.reflect.ClassTag
 
 import cats.Monad
@@ -118,8 +119,8 @@ abstract class ValueMapping[F[_]: Monad] extends Mapping[F] {
  case _ => false
  }
 
- def asList: Result[List[Cursor]] = (tpe, focus) match {
- case (ListType(tpe), it: List[_]) => it.map(f => mkChild(context.asType(tpe), f)).rightIor
+ def asList[C](factory: Factory[Cursor, C]): Result[C] = (tpe, focus) match {
+ case (ListType(tpe), it: List[_]) => it.view.map(f => mkChild(context.asType(tpe), f)).to(factory).rightIor
  case _ => mkErrorResult(s"Expected List type, found $tpe")
  }