From 858cca415c2994926e0d1ce13dca1ef58924892b Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Fri, 18 Aug 2023 17:22:00 +0100 Subject: [PATCH 01/29] Optimize List traverse for stack-safe Monads --- core/src/main/scala/cats/instances/list.scala | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/cats/instances/list.scala b/core/src/main/scala/cats/instances/list.scala index f4d01cda86..4eb2e0ba83 100644 --- a/core/src/main/scala/cats/instances/list.scala +++ b/core/src/main/scala/cats/instances/list.scala @@ -120,11 +120,25 @@ trait ListInstances extends cats.kernel.instances.ListInstances { def traverse[G[_], A, B](fa: List[A])(f: A => G[B])(implicit G: Applicative[G]): G[List[B]] = if (fa.isEmpty) G.pure(Nil) else - G.map(Chain.traverseViaChain { - val as = collection.mutable.ArrayBuffer[A]() - as ++= fa - wrapMutableIndexedSeq(as) - }(f))(_.toList) + G match { + case x: StackSafeMonad[G] => traverseDirectly[G, A, B](fa)(f)(x) + case _ => + G.map(Chain.traverseViaChain { + val as = collection.mutable.ArrayBuffer[A]() + as ++= fa + wrapMutableIndexedSeq(as) + }(f))(_.toList) + } + + private def traverseDirectly[G[_], A, B](fa: List[A])(f: A => G[B])(implicit G: StacksafeMonad[G]): G[List[B]] = + G.map(fa.foldLeft(G.pure(ListBuffer.empty[B])) { case (accG, a) => + G.flatMap(accG) { acc => + G.map(f(a)) { a => + acc += a + acc + } + } + })(_.toList) /** * This avoids making a very deep stack by building a tree instead From 847df54d8f7b1cf2b81c73d29791b021846aea05 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Mon, 21 Aug 2023 16:43:50 +0100 Subject: [PATCH 02/29] Optimize {List, Vector} traverse for StackSafeMonads --- core/src/main/scala/cats/Traverse.scala | 14 ++++++++++++++ core/src/main/scala/cats/instances/list.scala | 13 ++----------- core/src/main/scala/cats/instances/vector.scala | 5 ++++- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index f0f0ac0a17..3281c411bf 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -23,6 +23,8 @@ package cats import cats.data.State import cats.data.StateT +import cats.StackSafeMonad +import scala.collection.mutable /** * Traverse, also known as Traversable. @@ -284,4 +286,16 @@ object Traverse { @deprecated("Use cats.syntax object imports", "2.2.0") object nonInheritedOps extends ToTraverseOps + private[cats] def traverseDirectly[Coll[x] <: IterableOnce[x], G[_], A, B]( + builder: mutable.Builder[B, Coll[B]] + )(fa: Coll[A])(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Coll[B]] = + G.map(fa.iterator.foldLeft(G.pure(builder)) { case (accG, a) => + G.flatMap(accG) { acc => + G.map(f(a)) { a => + acc += a + acc + } + } + })(_.result()) + } diff --git a/core/src/main/scala/cats/instances/list.scala b/core/src/main/scala/cats/instances/list.scala index 4eb2e0ba83..87becaa369 100644 --- a/core/src/main/scala/cats/instances/list.scala +++ b/core/src/main/scala/cats/instances/list.scala @@ -23,6 +23,7 @@ package cats package instances import cats.data.{Chain, Ior, ZipList} +import cats.StackSafeMonad import cats.instances.StaticMethods.appendAll import cats.kernel.compat.scalaVersionSpecific._ import cats.kernel.instances.StaticMethods.wrapMutableIndexedSeq @@ -121,7 +122,7 @@ trait ListInstances extends cats.kernel.instances.ListInstances { if (fa.isEmpty) G.pure(Nil) else G match { - case x: StackSafeMonad[G] => traverseDirectly[G, A, B](fa)(f)(x) + case x: StackSafeMonad[G] => Traverse.traverseDirectly[List, G, A, B](ListBuffer.empty[B])(fa)(f)(x) case _ => G.map(Chain.traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() @@ -130,16 +131,6 @@ trait ListInstances extends cats.kernel.instances.ListInstances { }(f))(_.toList) } - private def traverseDirectly[G[_], A, B](fa: List[A])(f: A => G[B])(implicit G: StacksafeMonad[G]): G[List[B]] = - G.map(fa.foldLeft(G.pure(ListBuffer.empty[B])) { case (accG, a) => - G.flatMap(accG) { acc => - G.map(f(a)) { a => - acc += a - acc - } - } - })(_.toList) - /** * This avoids making a very deep stack by building a tree instead */ diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index 88db57d2b4..c7870f9cf5 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -125,7 +125,10 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances { } final override def traverse[G[_], A, B](fa: Vector[A])(f: A => G[B])(implicit G: Applicative[G]): G[Vector[B]] = - G.map(Chain.traverseViaChain(fa)(f))(_.toVector) + G match { + case x: StackSafeMonad[G] => Traverse.traverseDirectly(Vector.newBuilder[B])(fa)(f)(x) + case _ => G.map(Chain.traverseViaChain(fa)(f))(_.toVector) + } final override def updated_[A, B >: A](fa: Vector[A], idx: Long, b: B): Option[Vector[B]] = if (idx >= 0L && idx < fa.size.toLong) { From 878c4d0519571ca8d02efe6c4a015aaa9a5910e8 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Mon, 21 Aug 2023 17:04:53 +0100 Subject: [PATCH 03/29] Optimize {Queue, Map} traverse for StackSafeMonads --- core/src/main/scala/cats/instances/map.scala | 18 +++++++++++++++--- core/src/main/scala/cats/instances/queue.scala | 16 ++++++++++------ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/core/src/main/scala/cats/instances/map.scala b/core/src/main/scala/cats/instances/map.scala index 82460f4879..433d06aedf 100644 --- a/core/src/main/scala/cats/instances/map.scala +++ b/core/src/main/scala/cats/instances/map.scala @@ -44,9 +44,21 @@ trait MapInstances extends cats.kernel.instances.MapInstances { )(f: A => G[B])(implicit G: CommutativeApplicative[G]): G[Map[K, B]] = { if (fa.isEmpty) G.pure(Map.empty[K, B]) else - G.map(Chain.traverseViaChain(fa.toIndexedSeq) { case (k, a) => - G.map(f(a))((k, _)) - })(_.iterator.toMap) + G match { + case x: StackSafeMonad[G] => + x.map(fa.foldLeft(G.pure(Map.newBuilder[K, B])) { case (accG, (k, a)) => + x.flatMap(accG) { acc => + G.map(f(a)) { a => + acc += k -> a + acc + } + } + })(_.result()) + case _ => + G.map(Chain.traverseViaChain(fa.toIndexedSeq) { case (k, a) => + G.map(f(a))((k, _)) + })(_.iterator.toMap) + } } override def map[A, B](fa: Map[K, A])(f: A => B): Map[K, B] = diff --git a/core/src/main/scala/cats/instances/queue.scala b/core/src/main/scala/cats/instances/queue.scala index 1bfbc52af6..765a91f4aa 100644 --- a/core/src/main/scala/cats/instances/queue.scala +++ b/core/src/main/scala/cats/instances/queue.scala @@ -121,12 +121,16 @@ trait QueueInstances extends cats.kernel.instances.QueueInstances { def traverse[G[_], A, B](fa: Queue[A])(f: A => G[B])(implicit G: Applicative[G]): G[Queue[B]] = if (fa.isEmpty) G.pure(Queue.empty[B]) else - G.map(Chain.traverseViaChain { - val as = collection.mutable.ArrayBuffer[A]() - as ++= fa - wrapMutableIndexedSeq(as) - }(f)) { chain => - chain.foldLeft(Queue.empty[B])(_ :+ _) + G match { + case x: StackSafeMonad[G] => Traverse.traverseDirectly(Queue.newBuilder[B])(fa)(f)(x) + case _ => + G.map(Chain.traverseViaChain { + val as = collection.mutable.ArrayBuffer[A]() + as ++= fa + wrapMutableIndexedSeq(as) + }(f)) { chain => + chain.foldLeft(Queue.empty[B])(_ :+ _) + } } override def mapAccumulate[S, A, B](init: S, fa: Queue[A])(f: (S, A) => (S, B)): (S, Queue[B]) = From 9d86c9828429550a6d0c7410ab7c1328111619d4 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Mon, 21 Aug 2023 17:18:27 +0100 Subject: [PATCH 04/29] Optimize Seq traverse for StackSafeMonads --- core/src/main/scala/cats/instances/seq.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/cats/instances/seq.scala b/core/src/main/scala/cats/instances/seq.scala index cdbf0adf27..d69caea9d8 100644 --- a/core/src/main/scala/cats/instances/seq.scala +++ b/core/src/main/scala/cats/instances/seq.scala @@ -127,7 +127,12 @@ trait SeqInstances extends cats.kernel.instances.SeqInstances { } final override def traverse[G[_], A, B](fa: Seq[A])(f: A => G[B])(implicit G: Applicative[G]): G[Seq[B]] = - G.map(Chain.traverseViaChain(fa.toIndexedSeq)(f))(_.toVector) + G match { + case x: StackSafeMonad[G] => + Traverse.traverseDirectly(Seq.newBuilder[B])(fa)(f)(x) + case _ => + G.map(Chain.traverseViaChain(fa.toIndexedSeq)(f))(_.toVector) + } override def mapWithIndex[A, B](fa: Seq[A])(f: (A, Int) => B): Seq[B] = fa.zipWithIndex.map(ai => f(ai._1, ai._2)) From c4bcba225a9725b8dd1feb05520f5fede471a31e Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Tue, 22 Aug 2023 12:26:09 +0100 Subject: [PATCH 05/29] Optimize ArraySeq traverse for StackSafeMonads --- core/src/main/scala-2.13+/cats/instances/arraySeq.scala | 8 +++++++- core/src/main/scala/cats/Traverse.scala | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala index 355664b0be..48bfa78185 100644 --- a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala +++ b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala @@ -102,7 +102,13 @@ private[cats] object ArraySeqInstances { B.combineAll(fa.iterator.map(f)) def traverse[G[_], A, B](fa: ArraySeq[A])(f: A => G[B])(implicit G: Applicative[G]): G[ArraySeq[B]] = - G.map(Chain.traverseViaChain(fa)(f))(_.iterator.to(ArraySeq.untagged)) + G match { + case x: StackSafeMonad[G] => + x.map(Traverse.traverseDirectly(Vector.newBuilder[B])(fa.iterator)(f)(x))(_.to(ArraySeq.untagged)) + case _ => + G.map(Chain.traverseViaChain(fa)(f))(_.iterator.to(ArraySeq.untagged)) + + } override def mapAccumulate[S, A, B](init: S, fa: ArraySeq[A])(f: (S, A) => (S, B)): (S, ArraySeq[B]) = StaticMethods.mapAccumulateFromStrictFunctor(init, fa, f)(this) diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index 3281c411bf..f7ae38f588 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -288,7 +288,7 @@ object Traverse { private[cats] def traverseDirectly[Coll[x] <: IterableOnce[x], G[_], A, B]( builder: mutable.Builder[B, Coll[B]] - )(fa: Coll[A])(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Coll[B]] = + )(fa: IterableOnce[A])(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Coll[B]] = G.map(fa.iterator.foldLeft(G.pure(builder)) { case (accG, a) => G.flatMap(accG) { acc => G.map(f(a)) { a => From c4c64eccd56e976b3c008a2303165b24b6f956da Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Tue, 22 Aug 2023 14:21:48 +0100 Subject: [PATCH 06/29] Optimize Chain traverse for StackSafeMonads --- core/src/main/scala/cats/data/Chain.scala | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/cats/data/Chain.scala b/core/src/main/scala/cats/data/Chain.scala index 01bda7592d..ba479021a2 100644 --- a/core/src/main/scala/cats/data/Chain.scala +++ b/core/src/main/scala/cats/data/Chain.scala @@ -1241,11 +1241,16 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { def traverse[G[_], A, B](fa: Chain[A])(f: A => G[B])(implicit G: Applicative[G]): G[Chain[B]] = if (fa.isEmpty) G.pure(Chain.nil) else - traverseViaChain { - val as = collection.mutable.ArrayBuffer[A]() - as ++= fa.iterator - KernelStaticMethods.wrapMutableIndexedSeq(as) - }(f) + G match { + case x: StackSafeMonad[G] => + x.map(Traverse.traverseDirectly(List.newBuilder[B])(fa.iterator)(f)(x))(Chain.fromSeq(_)) + case _ => + traverseViaChain { + val as = collection.mutable.ArrayBuffer[A]() + as ++= fa.iterator + KernelStaticMethods.wrapMutableIndexedSeq(as) + }(f) + } override def mapAccumulate[S, A, B](init: S, fa: Chain[A])(f: (S, A) => (S, B)): (S, Chain[B]) = StaticMethods.mapAccumulateFromStrictFunctor(init, fa, f)(this) From 1344a709cec0519591e7af8b0c44a6b2b6d6c08e Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Tue, 22 Aug 2023 15:48:05 +0100 Subject: [PATCH 07/29] Add size hint to StackSafeMonad traverse --- core/src/main/scala/cats/Traverse.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index f7ae38f588..fbd4e4bd9b 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -288,7 +288,11 @@ object Traverse { private[cats] def traverseDirectly[Coll[x] <: IterableOnce[x], G[_], A, B]( builder: mutable.Builder[B, Coll[B]] - )(fa: IterableOnce[A])(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Coll[B]] = + )(fa: IterableOnce[A])(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Coll[B]] = { + val size = fa.knownSize + if (size >= 0) { + builder.sizeHint(size) + } G.map(fa.iterator.foldLeft(G.pure(builder)) { case (accG, a) => G.flatMap(accG) { acc => G.map(f(a)) { a => @@ -297,5 +301,6 @@ object Traverse { } } })(_.result()) + } } From b6973dc467bcf3a388d1d8c62b60c21c1cfa49e3 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Tue, 22 Aug 2023 16:22:26 +0100 Subject: [PATCH 08/29] Optimize List traverseFilter for StackSafeMonads --- core/src/main/scala/cats/TraverseFilter.scala | 18 ++++++++++++++++++ core/src/main/scala/cats/instances/list.scala | 14 +++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/cats/TraverseFilter.scala b/core/src/main/scala/cats/TraverseFilter.scala index 2f44b0f7c3..418916582b 100644 --- a/core/src/main/scala/cats/TraverseFilter.scala +++ b/core/src/main/scala/cats/TraverseFilter.scala @@ -24,6 +24,7 @@ package cats import cats.data.State import scala.collection.immutable.{IntMap, TreeSet} +import scala.collection.mutable /** * `TraverseFilter`, also known as `Witherable`, represents list-like structures @@ -203,4 +204,21 @@ object TraverseFilter { @deprecated("Use cats.syntax object imports", "2.2.0") object nonInheritedOps extends ToTraverseFilterOps + private[cats] def traverseFilterDirectly[Coll[x] <: IterableOnce[x], G[_], A, B]( + builder: mutable.Builder[B, Coll[B]] + )(fa: IterableOnce[A])(f: A => G[Option[B]])(implicit G: StackSafeMonad[G]): G[Coll[B]] = { + val size = fa.knownSize + if (size >= 0) { + builder.sizeHint(size) + } + G.map(fa.iterator.foldLeft(G.pure(builder)) { case (bldrG, a) => + G.flatMap(bldrG) { bldr => + G.map(f(a)) { + case Some(b) => bldr += b + case None => bldr + } + } + })(_.result()) + } + } diff --git a/core/src/main/scala/cats/instances/list.scala b/core/src/main/scala/cats/instances/list.scala index 87becaa369..09fc56b6da 100644 --- a/core/src/main/scala/cats/instances/list.scala +++ b/core/src/main/scala/cats/instances/list.scala @@ -315,11 +315,15 @@ private[instances] trait ListInstancesBinCompat0 { def traverseFilter[G[_], A, B](fa: List[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[List[B]] = if (fa.isEmpty) G.pure(Nil) else - G.map(Chain.traverseFilterViaChain { - val as = collection.mutable.ArrayBuffer[A]() - as ++= fa - wrapMutableIndexedSeq(as) - }(f))(_.toList) + G match { + case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(List.newBuilder[B])(fa)(f)(x) + case _ => + G.map(Chain.traverseFilterViaChain { + val as = collection.mutable.ArrayBuffer[A]() + as ++= fa + wrapMutableIndexedSeq(as) + }(f))(_.toList) + } override def filterA[G[_], A](fa: List[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[List[A]] = traverse From f0648c40f84581ac2be9db50948fda9df00aa03d Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Tue, 22 Aug 2023 16:39:38 +0100 Subject: [PATCH 09/29] Optimize {Seq, Queue, Vector, ArraySeq} traverseFilter for StackSafeMonads --- .../scala-2.13+/cats/instances/arraySeq.scala | 14 +++++++++++--- core/src/main/scala/cats/instances/queue.scala | 16 ++++++++++------ core/src/main/scala/cats/instances/seq.scala | 6 +++++- core/src/main/scala/cats/instances/vector.scala | 6 +++++- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala index 48bfa78185..a4afc5bf42 100644 --- a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala +++ b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala @@ -220,9 +220,17 @@ private[cats] object ArraySeqInstances { def traverseFilter[G[_], A, B]( fa: ArraySeq[A] )(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[ArraySeq[B]] = - fa.foldRight(Eval.now(G.pure(ArraySeq.untagged.empty[B]))) { case (x, xse) => - G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o)) - }.value + G match { + case x: StackSafeMonad[G] => + x.map(TraverseFilter.traverseFilterDirectly(Vector.newBuilder[B])(fa.iterator)(f)(x))( + _.to(ArraySeq.untagged) + ) + case _ => + fa.foldRight(Eval.now(G.pure(ArraySeq.untagged.empty[B]))) { case (x, xse) => + G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o)) + }.value + + } override def filterA[G[_], A](fa: ArraySeq[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[ArraySeq[A]] = fa.foldRight(Eval.now(G.pure(ArraySeq.untagged.empty[A]))) { case (x, xse) => diff --git a/core/src/main/scala/cats/instances/queue.scala b/core/src/main/scala/cats/instances/queue.scala index 765a91f4aa..8ef5c2dd70 100644 --- a/core/src/main/scala/cats/instances/queue.scala +++ b/core/src/main/scala/cats/instances/queue.scala @@ -227,12 +227,16 @@ private object QueueInstances { def traverseFilter[G[_], A, B](fa: Queue[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Queue[B]] = if (fa.isEmpty) G.pure(Queue.empty[B]) else - G.map(Chain.traverseFilterViaChain { - val as = collection.mutable.ArrayBuffer[A]() - as ++= fa - wrapMutableIndexedSeq(as) - }(f)) { chain => - chain.foldLeft(Queue.empty[B])(_ :+ _) + G match { + case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(Queue.newBuilder[B])(fa)(f)(x) + case _ => + G.map(Chain.traverseFilterViaChain { + val as = collection.mutable.ArrayBuffer[A]() + as ++= fa + wrapMutableIndexedSeq(as) + }(f)) { chain => + chain.foldLeft(Queue.empty[B])(_ :+ _) + } } override def filterA[G[_], A](fa: Queue[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[Queue[A]] = diff --git a/core/src/main/scala/cats/instances/seq.scala b/core/src/main/scala/cats/instances/seq.scala index d69caea9d8..ec7a823b9e 100644 --- a/core/src/main/scala/cats/instances/seq.scala +++ b/core/src/main/scala/cats/instances/seq.scala @@ -198,7 +198,11 @@ trait SeqInstances extends cats.kernel.instances.SeqInstances { override def flattenOption[A](fa: Seq[Option[A]]): Seq[A] = fa.flatten def traverseFilter[G[_], A, B](fa: Seq[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Seq[B]] = - G.map(Chain.traverseFilterViaChain(fa.toIndexedSeq)(f))(_.toVector) + G match { + case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(Seq.newBuilder[B])(fa)(f)(x) + case _ => + G.map(Chain.traverseFilterViaChain(fa.toIndexedSeq)(f))(_.toVector) + } override def filterA[G[_], A](fa: Seq[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[Seq[A]] = traverse diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index c7870f9cf5..1186546de1 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -266,7 +266,11 @@ private[instances] trait VectorInstancesBinCompat0 { override def flattenOption[A](fa: Vector[Option[A]]): Vector[A] = fa.flatten def traverseFilter[G[_], A, B](fa: Vector[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Vector[B]] = - G.map(Chain.traverseFilterViaChain(fa)(f))(_.toVector) + G match { + case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(Vector.newBuilder[B])(fa)(f)(x) + case _ => + G.map(Chain.traverseFilterViaChain(fa)(f))(_.toVector) + } override def filterA[G[_], A](fa: Vector[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[Vector[A]] = traverse From dbccc8ef5f2f0009f96ca090effc8ed92c179cfa Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Tue, 22 Aug 2023 16:44:04 +0100 Subject: [PATCH 10/29] Optimize Chain filterTraverse for StackSafeMonads --- core/src/main/scala/cats/data/Chain.scala | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/cats/data/Chain.scala b/core/src/main/scala/cats/data/Chain.scala index ba479021a2..703c758b86 100644 --- a/core/src/main/scala/cats/data/Chain.scala +++ b/core/src/main/scala/cats/data/Chain.scala @@ -1359,11 +1359,16 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { def traverseFilter[G[_], A, B](fa: Chain[A])(f: A => G[Option[B]])(implicit G: Applicative[G]): G[Chain[B]] = if (fa.isEmpty) G.pure(Chain.nil) else - traverseFilterViaChain { - val as = collection.mutable.ArrayBuffer[A]() - as ++= fa.iterator - KernelStaticMethods.wrapMutableIndexedSeq(as) - }(f) + G match { + case x: StackSafeMonad[G] => + G.map(TraverseFilter.traverseFilterDirectly(List.newBuilder[B])(fa.iterator)(f)(x))(Chain.fromSeq(_)) + case _ => + traverseFilterViaChain { + val as = collection.mutable.ArrayBuffer[A]() + as ++= fa.iterator + KernelStaticMethods.wrapMutableIndexedSeq(as) + }(f) + } override def filterA[G[_], A](fa: Chain[A])(f: A => G[Boolean])(implicit G: Applicative[G]): G[Chain[A]] = traverse From fbf5ea56896b9c5ae99b5c2c1eb50f4c14b5a6b8 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Tue, 22 Aug 2023 17:03:04 +0100 Subject: [PATCH 11/29] Optimize List traverse_ for StackSafeMonads --- core/src/main/scala/cats/Traverse.scala | 14 ++++ core/src/main/scala/cats/instances/list.scala | 68 ++++++++++--------- 2 files changed, 50 insertions(+), 32 deletions(-) diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index fbd4e4bd9b..1cdb607cd3 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -303,4 +303,18 @@ object Traverse { })(_.result()) } + private[cats] def traverse_Directly[G[_], A, B]( + fa: IterableOnce[A] + )(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Unit] = { + val iter = fa.iterator + if (iter.hasNext) { + val first = iter.next() + G.map(iter.foldLeft(f(first)) { case (g, a) => + G.flatMap(g) { _ => + f(a) + } + })(_ => ()) + } else G.unit + } + } diff --git a/core/src/main/scala/cats/instances/list.scala b/core/src/main/scala/cats/instances/list.scala index 09fc56b6da..2b18ee6810 100644 --- a/core/src/main/scala/cats/instances/list.scala +++ b/core/src/main/scala/cats/instances/list.scala @@ -135,40 +135,44 @@ trait ListInstances extends cats.kernel.instances.ListInstances { * This avoids making a very deep stack by building a tree instead */ override def traverse_[G[_], A, B](fa: List[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = { - // the cost of this is O(size log size) - // c(n) = n + 2 * c(n/2) = n + 2(n/2 log (n/2)) = n + n (logn - 1) = n log n - // invariant: size >= 1 - def runHalf(size: Int, fa: List[A]): Eval[G[Unit]] = - if (size > 1) { - val leftSize = size / 2 - val rightSize = size - leftSize - val (leftL, rightL) = fa.splitAt(leftSize) - runHalf(leftSize, leftL) - .flatMap { left => - val right = runHalf(rightSize, rightL) - G.map2Eval(left, right) { (_, _) => () } + G match { + case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) + case _ => + // the cost of this is O(size log size) + // c(n) = n + 2 * c(n/2) = n + 2(n/2 log (n/2)) = n + n (logn - 1) = n log n + // invariant: size >= 1 + def runHalf(size: Int, fa: List[A]): Eval[G[Unit]] = + if (size > 1) { + val leftSize = size / 2 + val rightSize = size - leftSize + val (leftL, rightL) = fa.splitAt(leftSize) + runHalf(leftSize, leftL) + .flatMap { left => + val right = runHalf(rightSize, rightL) + G.map2Eval(left, right) { (_, _) => () } + } + } else { + // avoid pattern matching when we know that there is only one element + val a = fa.head + // we evaluate this at most one time, + // always is a bit cheaper in such cases + // + // Here is the point of the laziness using Eval: + // we avoid calling f(a) or G.void in the + // event that the computation has already + // failed. We do not use laziness to avoid + // traversing fa, which we will do fully + // in all cases. + Eval.always { + val gb = f(a) + G.void(gb) + } } - } else { - // avoid pattern matching when we know that there is only one element - val a = fa.head - // we evaluate this at most one time, - // always is a bit cheaper in such cases - // - // Here is the point of the laziness using Eval: - // we avoid calling f(a) or G.void in the - // event that the computation has already - // failed. We do not use laziness to avoid - // traversing fa, which we will do fully - // in all cases. - Eval.always { - val gb = f(a) - G.void(gb) - } - } - val len = fa.length - if (len == 0) G.unit - else runHalf(len, fa).value + val len = fa.length + if (len == 0) G.unit + else runHalf(len, fa).value + } } def functor: Functor[List] = this From 9f60d2bfd881ac57955e038589d31db8953e5b53 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Wed, 23 Aug 2023 14:09:04 +0100 Subject: [PATCH 12/29] Optimize Vector traverse_ for StackSafeMonads Also fix 2.12 errors --- core/src/main/scala/cats/Traverse.scala | 1 + core/src/main/scala/cats/TraverseFilter.scala | 1 + .../main/scala/cats/instances/vector.scala | 64 ++++++++++--------- .../kernel/compat/scalaVersionSpecific.scala | 2 + 4 files changed, 38 insertions(+), 30 deletions(-) diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index 1cdb607cd3..9109a860e1 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -23,6 +23,7 @@ package cats import cats.data.State import cats.data.StateT +import cats.kernel.compat.scalaVersionSpecific._ import cats.StackSafeMonad import scala.collection.mutable diff --git a/core/src/main/scala/cats/TraverseFilter.scala b/core/src/main/scala/cats/TraverseFilter.scala index 418916582b..2cfa2145cb 100644 --- a/core/src/main/scala/cats/TraverseFilter.scala +++ b/core/src/main/scala/cats/TraverseFilter.scala @@ -22,6 +22,7 @@ package cats import cats.data.State +import cats.kernel.compat.scalaVersionSpecific._ import scala.collection.immutable.{IntMap, TreeSet} import scala.collection.mutable diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index 1186546de1..d47d5f010a 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -141,38 +141,42 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances { * This avoids making a very deep stack by building a tree instead */ override def traverse_[G[_], A, B](fa: Vector[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = { - // the cost of this is O(size) - // c(n) = 1 + 2 * c(n/2) - // invariant: size >= 1 - def runHalf(size: Int, idx: Int): Eval[G[Unit]] = - if (size > 1) { - val leftSize = size / 2 - val rightSize = size - leftSize - runHalf(leftSize, idx) - .flatMap { left => - val right = runHalf(rightSize, idx + leftSize) - G.map2Eval(left, right) { (_, _) => () } + G match { + case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) + case _ => + // the cost of this is O(size) + // c(n) = 1 + 2 * c(n/2) + // invariant: size >= 1 + def runHalf(size: Int, idx: Int): Eval[G[Unit]] = + if (size > 1) { + val leftSize = size / 2 + val rightSize = size - leftSize + runHalf(leftSize, idx) + .flatMap { left => + val right = runHalf(rightSize, idx + leftSize) + G.map2Eval(left, right) { (_, _) => () } + } + } else { + val a = fa(idx) + // we evaluate this at most one time, + // always is a bit cheaper in such cases + // + // Here is the point of the laziness using Eval: + // we avoid calling f(a) or G.void in the + // event that the computation has already + // failed. We do not use laziness to avoid + // traversing fa, which we will do fully + // in all cases. + Eval.always { + val gb = f(a) + G.void(gb) + } } - } else { - val a = fa(idx) - // we evaluate this at most one time, - // always is a bit cheaper in such cases - // - // Here is the point of the laziness using Eval: - // we avoid calling f(a) or G.void in the - // event that the computation has already - // failed. We do not use laziness to avoid - // traversing fa, which we will do fully - // in all cases. - Eval.always { - val gb = f(a) - G.void(gb) - } - } - val len = fa.length - if (len == 0) G.unit - else runHalf(len, 0).value + val len = fa.length + if (len == 0) G.unit + else runHalf(len, 0).value + } } override def mapAccumulate[S, A, B](init: S, fa: Vector[A])(f: (S, A) => (S, B)): (S, Vector[B]) = diff --git a/kernel/src/main/scala-2.12/cats/kernel/compat/scalaVersionSpecific.scala b/kernel/src/main/scala-2.12/cats/kernel/compat/scalaVersionSpecific.scala index ee73d39ae2..ea473c1350 100644 --- a/kernel/src/main/scala-2.12/cats/kernel/compat/scalaVersionSpecific.scala +++ b/kernel/src/main/scala-2.12/cats/kernel/compat/scalaVersionSpecific.scala @@ -34,6 +34,8 @@ private[cats] object scalaVersionSpecific { implicit class traversableOnceExtension[A](private val to: TraversableOnce[A]) extends AnyVal { def iterator: Iterator[A] = to.toIterator + + def knownSize: Int = -1 } implicit class doubleExtension(private val double: Double) extends AnyVal { From f4fcf57efa41679b4d687c3df8623cc37d827be1 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Wed, 23 Aug 2023 16:36:05 +0100 Subject: [PATCH 13/29] Optimize {Seq, ArraySeq, Chain, Queue} traverse_ for StackSafeMonads --- .../main/scala-2.13+/cats/instances/arraySeq.scala | 11 +++++++++++ core/src/main/scala/cats/data/Chain.scala | 11 +++++++++++ core/src/main/scala/cats/instances/queue.scala | 11 +++++++++++ core/src/main/scala/cats/instances/seq.scala | 11 +++++++++++ 4 files changed, 44 insertions(+) diff --git a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala index a4afc5bf42..4ced92e1cb 100644 --- a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala +++ b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala @@ -110,6 +110,17 @@ private[cats] object ArraySeqInstances { } + override def traverse_[G[_], A, B](fa: ArraySeq[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = + G match { + case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) + case _ => + foldRight(fa, Always(G.pure(()))) { (a, acc) => + G.map2Eval(f(a), acc) { (_, _) => + () + } + }.value + } + override def mapAccumulate[S, A, B](init: S, fa: ArraySeq[A])(f: (S, A) => (S, B)): (S, ArraySeq[B]) = StaticMethods.mapAccumulateFromStrictFunctor(init, fa, f)(this) diff --git a/core/src/main/scala/cats/data/Chain.scala b/core/src/main/scala/cats/data/Chain.scala index 703c758b86..4fe4ec0cbe 100644 --- a/core/src/main/scala/cats/data/Chain.scala +++ b/core/src/main/scala/cats/data/Chain.scala @@ -1252,6 +1252,17 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { }(f) } + override def traverse_[G[_], A, B](fa: Chain[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = + G match { + case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa.iterator)(f)(x) + case _ => + foldRight(fa, Always(G.pure(()))) { (a, acc) => + G.map2Eval(f(a), acc) { (_, _) => + () + } + }.value + } + override def mapAccumulate[S, A, B](init: S, fa: Chain[A])(f: (S, A) => (S, B)): (S, Chain[B]) = StaticMethods.mapAccumulateFromStrictFunctor(init, fa, f)(this) diff --git a/core/src/main/scala/cats/instances/queue.scala b/core/src/main/scala/cats/instances/queue.scala index 8ef5c2dd70..75aaf32a9d 100644 --- a/core/src/main/scala/cats/instances/queue.scala +++ b/core/src/main/scala/cats/instances/queue.scala @@ -133,6 +133,17 @@ trait QueueInstances extends cats.kernel.instances.QueueInstances { } } + override def traverse_[G[_], A, B](fa: Queue[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = + G match { + case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) + case _ => + foldRight(fa, Always(G.pure(()))) { (a, acc) => + G.map2Eval(f(a), acc) { (_, _) => + () + } + }.value + } + override def mapAccumulate[S, A, B](init: S, fa: Queue[A])(f: (S, A) => (S, B)): (S, Queue[B]) = StaticMethods.mapAccumulateFromStrictFunctor(init, fa, f)(this) diff --git a/core/src/main/scala/cats/instances/seq.scala b/core/src/main/scala/cats/instances/seq.scala index ec7a823b9e..005139d6c2 100644 --- a/core/src/main/scala/cats/instances/seq.scala +++ b/core/src/main/scala/cats/instances/seq.scala @@ -134,6 +134,17 @@ trait SeqInstances extends cats.kernel.instances.SeqInstances { G.map(Chain.traverseViaChain(fa.toIndexedSeq)(f))(_.toVector) } + override def traverse_[G[_], A, B](fa: Seq[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = + G match { + case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) + case _ => + foldRight(fa, Always(G.pure(()))) { (a, acc) => + G.map2Eval(f(a), acc) { (_, _) => + () + } + }.value + } + override def mapWithIndex[A, B](fa: Seq[A])(f: (A, Int) => B): Seq[B] = fa.zipWithIndex.map(ai => f(ai._1, ai._2)) From d50bf8341da6e1bd49c5bde046e0cbada2c94675 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Wed, 23 Aug 2023 16:58:01 +0100 Subject: [PATCH 14/29] Add extra laws tests so we test the multiple branches corresponding to different runtime types for the Applicative instance --- .../src/test/scala-2.13+/cats/tests/ArraySeqSuite.scala | 4 +++- tests/shared/src/test/scala/cats/tests/ChainSuite.scala | 2 ++ tests/shared/src/test/scala/cats/tests/ListSuite.scala | 4 +++- tests/shared/src/test/scala/cats/tests/MapSuite.scala | 4 +++- tests/shared/src/test/scala/cats/tests/QueueSuite.scala | 4 +++- tests/shared/src/test/scala/cats/tests/SeqSuite.scala | 4 +++- tests/shared/src/test/scala/cats/tests/VectorSuite.scala | 4 +++- 7 files changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/shared/src/test/scala-2.13+/cats/tests/ArraySeqSuite.scala b/tests/shared/src/test/scala-2.13+/cats/tests/ArraySeqSuite.scala index 9c77c3cbdf..2d1b3296de 100644 --- a/tests/shared/src/test/scala-2.13+/cats/tests/ArraySeqSuite.scala +++ b/tests/shared/src/test/scala-2.13+/cats/tests/ArraySeqSuite.scala @@ -21,7 +21,7 @@ package cats.tests -import cats.{Align, Alternative, CoflatMap, Monad, MonoidK, Traverse, TraverseFilter} +import cats.{Align, Alternative, CoflatMap, Eval, Monad, MonoidK, Traverse, TraverseFilter} import cats.kernel.{Eq, Hash, Monoid, Order, PartialOrder} import cats.kernel.laws.discipline.{EqTests, HashTests, MonoidTests, OrderTests, PartialOrderTests} import cats.laws.discipline.{ @@ -52,7 +52,9 @@ class ArraySeqSuite extends CatsSuite { checkAll("ArraySeq[Int]", AlternativeTests[ArraySeq].alternative[Int, Int, Int]) checkAll("Alternative[ArraySeq]", SerializableTests.serializable(Alternative[ArraySeq])) + // Traverse behaviour discriminates on the Runtime type of the Applicative checkAll("ArraySeq[Int] with Option", TraverseTests[ArraySeq].traverse[Int, Int, Int, Set[Int], Option, Option]) + checkAll("ArraySeq[Int] with Eval", TraverseTests[ArraySeq].traverse[Int, Int, Int, Set[Int], Eval, Eval]) checkAll("Traverse[ArraySeq]", SerializableTests.serializable(Traverse[ArraySeq])) checkAll("ArraySeq[Int]", MonadTests[ArraySeq].monad[Int, Int, Int]) diff --git a/tests/shared/src/test/scala/cats/tests/ChainSuite.scala b/tests/shared/src/test/scala/cats/tests/ChainSuite.scala index ad5d593aa0..4a920ad4c4 100644 --- a/tests/shared/src/test/scala/cats/tests/ChainSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/ChainSuite.scala @@ -43,7 +43,9 @@ class ChainSuite extends CatsSuite { checkAll("Chain[Int]", AlternativeTests[Chain].alternative[Int, Int, Int]) checkAll("Alternative[Chain]", SerializableTests.serializable(Alternative[Chain])) + // Traverse behaviour discriminates on the Runtime type of the Applicative checkAll("Chain[Int] with Option", TraverseTests[Chain].traverse[Int, Int, Int, Set[Int], Option, Option]) + checkAll("Chain[Int] with Eval", TraverseTests[Chain].traverse[Int, Int, Int, Set[Int], Eval, Eval]) checkAll("Traverse[Chain]", SerializableTests.serializable(Traverse[Chain])) checkAll("Chain[Int]", MonadTests[Chain].monad[Int, Int, Int]) diff --git a/tests/shared/src/test/scala/cats/tests/ListSuite.scala b/tests/shared/src/test/scala/cats/tests/ListSuite.scala index 8bb06f2d22..d23c224fdb 100644 --- a/tests/shared/src/test/scala/cats/tests/ListSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/ListSuite.scala @@ -21,7 +21,7 @@ package cats.tests -import cats.{Align, Alternative, CoflatMap, Monad, Semigroupal, Traverse, TraverseFilter} +import cats.{Align, Alternative, CoflatMap, Eval, Monad, Semigroupal, Traverse, TraverseFilter} import cats.data.{NonEmptyList, ZipList} import cats.laws.discipline.{ AlignTests, @@ -52,7 +52,9 @@ class ListSuite extends CatsSuite { checkAll("List[Int]", AlternativeTests[List].alternative[Int, Int, Int]) checkAll("Alternative[List]", SerializableTests.serializable(Alternative[List])) + // Traverse behaviour discriminates on the Runtime type of the Applicative checkAll("List[Int] with Option", TraverseTests[List].traverse[Int, Int, Int, Set[Int], Option, Option]) + checkAll("List[Int] with Eval", TraverseTests[List].traverse[Int, Int, Int, Set[Int], Eval, Eval]) checkAll("Traverse[List]", SerializableTests.serializable(Traverse[List])) checkAll("List[Int]", MonadTests[List].monad[Int, Int, Int]) diff --git a/tests/shared/src/test/scala/cats/tests/MapSuite.scala b/tests/shared/src/test/scala/cats/tests/MapSuite.scala index 21b80e6b92..495ff4b015 100644 --- a/tests/shared/src/test/scala/cats/tests/MapSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/MapSuite.scala @@ -21,7 +21,7 @@ package cats.tests -import cats.{Align, FlatMap, FunctorFilter, MonoidK, Semigroupal, Show, UnorderedTraverse} +import cats.{Align, Eval, FlatMap, FunctorFilter, MonoidK, Semigroupal, Show, UnorderedTraverse} import cats.arrow.Compose import cats.kernel.{CommutativeMonoid, Monoid} import cats.kernel.instances.StaticMethods.wrapMutableMap @@ -48,9 +48,11 @@ class MapSuite extends CatsSuite { checkAll("Map[Int, Int]", FlatMapTests[Map[Int, *]].flatMap[Int, Int, Int]) checkAll("FlatMap[Map[Int, *]]", SerializableTests.serializable(FlatMap[Map[Int, *]])) + // Traverse behaviour discriminates on the Runtime type of the Applicative checkAll("Map[Int, Int] with Option", UnorderedTraverseTests[Map[Int, *]].unorderedTraverse[Int, Int, Int, Option, Option] ) + checkAll("Map[Int, Int] with Eval", UnorderedTraverseTests[Map[Int, *]].unorderedTraverse[Int, Int, Int, Eval, Eval]) checkAll("UnorderedTraverse[Map[Int, *]]", SerializableTests.serializable(UnorderedTraverse[Map[Int, *]])) checkAll("Map[Int, Int]", FunctorFilterTests[Map[Int, *]].functorFilter[Int, Int, Int]) diff --git a/tests/shared/src/test/scala/cats/tests/QueueSuite.scala b/tests/shared/src/test/scala/cats/tests/QueueSuite.scala index 670eac7a98..40ae944294 100644 --- a/tests/shared/src/test/scala/cats/tests/QueueSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/QueueSuite.scala @@ -21,7 +21,7 @@ package cats.tests -import cats.{Alternative, CoflatMap, Monad, Semigroupal, Traverse, TraverseFilter} +import cats.{Alternative, CoflatMap, Eval, Monad, Semigroupal, Traverse, TraverseFilter} import cats.laws.discipline.{ AlternativeTests, CoflatMapTests, @@ -50,7 +50,9 @@ class QueueSuite extends CatsSuite { checkAll("Queue[Int]", MonadTests[Queue].monad[Int, Int, Int]) checkAll("Monad[Queue]", SerializableTests.serializable(Monad[Queue])) + // Traverse behaviour discriminates on the Runtime type of the Applicative checkAll("Queue[Int] with Option", TraverseTests[Queue].traverse[Int, Int, Int, Set[Int], Option, Option]) + checkAll("Queue[Int] with Eval", TraverseTests[Queue].traverse[Int, Int, Int, Set[Int], Eval, Eval]) checkAll("Traverse[Queue]", SerializableTests.serializable(Traverse[Queue])) checkAll("Queue[Int]", TraverseFilterTests[Queue].traverseFilter[Int, Int, Int]) diff --git a/tests/shared/src/test/scala/cats/tests/SeqSuite.scala b/tests/shared/src/test/scala/cats/tests/SeqSuite.scala index e2d7106b5a..0d302fed15 100644 --- a/tests/shared/src/test/scala/cats/tests/SeqSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/SeqSuite.scala @@ -21,7 +21,7 @@ package cats.tests -import cats.{Align, Alternative, CoflatMap, Monad, Semigroupal, Traverse, TraverseFilter} +import cats.{Align, Alternative, CoflatMap, Eval, Monad, Semigroupal, Traverse, TraverseFilter} import cats.data.ZipSeq import cats.laws.discipline.{ AlignTests, @@ -51,7 +51,9 @@ class SeqSuite extends CatsSuite { checkAll("Seq[Int]", AlternativeTests[Seq].alternative[Int, Int, Int]) checkAll("Alternative[Seq]", SerializableTests.serializable(Alternative[Seq])) + // Traverse behaviour discriminates on the Runtime type of the Applicative checkAll("Seq[Int] with Option", TraverseTests[Seq].traverse[Int, Int, Int, Set[Int], Option, Option]) + checkAll("Seq[Int] with Eval", TraverseTests[Seq].traverse[Int, Int, Int, Set[Int], Eval, Eval]) checkAll("Traverse[Seq]", SerializableTests.serializable(Traverse[Seq])) checkAll("Seq[Int]", MonadTests[Seq].monad[Int, Int, Int]) diff --git a/tests/shared/src/test/scala/cats/tests/VectorSuite.scala b/tests/shared/src/test/scala/cats/tests/VectorSuite.scala index f4b91e7369..4777dab5b1 100644 --- a/tests/shared/src/test/scala/cats/tests/VectorSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/VectorSuite.scala @@ -21,7 +21,7 @@ package cats.tests -import cats.{Align, Alternative, CoflatMap, Monad, Semigroupal, Traverse, TraverseFilter} +import cats.{Align, Alternative, CoflatMap, Eval, Monad, Semigroupal, Traverse, TraverseFilter} import cats.data.{NonEmptyVector, ZipVector} import cats.laws.discipline.{ AlignTests, @@ -51,7 +51,9 @@ class VectorSuite extends CatsSuite { checkAll("Vector[Int]", AlternativeTests[Vector].alternative[Int, Int, Int]) checkAll("Alternative[Vector]", SerializableTests.serializable(Alternative[Vector])) + // TraverseFilter behaviour discriminates on the Runtime type of the Applicative checkAll("Vector[Int] with Option", TraverseTests[Vector].traverse[Int, Int, Int, Set[Int], Option, Option]) + checkAll("Vector[Int] with Eval", TraverseTests[Vector].traverse[Int, Int, Int, Set[Int], Eval, Eval]) checkAll("Traverse[Vector]", SerializableTests.serializable(Traverse[Vector])) checkAll("Vector[Int]", MonadTests[Vector].monad[Int, Int, Int]) From 5c5049fbcb42e087dbd5268214aeec8358f92c51 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Wed, 23 Aug 2023 17:04:34 +0100 Subject: [PATCH 15/29] Add benchmarks for traverse_ and for Chain --- .../main/scala/cats/bench/TraverseBench.scala | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/bench/src/main/scala/cats/bench/TraverseBench.scala b/bench/src/main/scala/cats/bench/TraverseBench.scala index dd2cc1e048..188b1f6a43 100644 --- a/bench/src/main/scala/cats/bench/TraverseBench.scala +++ b/bench/src/main/scala/cats/bench/TraverseBench.scala @@ -24,15 +24,19 @@ package cats.bench import cats.{Eval, Traverse, TraverseFilter} import org.openjdk.jmh.annotations.{Benchmark, Param, Scope, Setup, State} import org.openjdk.jmh.infra.Blackhole +import cats.data.Chain @State(Scope.Benchmark) class TraverseBench { val listT: Traverse[List] = Traverse[List] val listTFilter: TraverseFilter[List] = TraverseFilter[List] + val chainTFilter: TraverseFilter[Chain] = TraverseFilter[Chain] val vectorT: Traverse[Vector] = Traverse[Vector] val vectorTFilter: TraverseFilter[Vector] = TraverseFilter[Vector] + val chainT: Traverse[Chain] = Traverse[Chain] + // the unit of CPU work per iteration private[this] val Work: Long = 10 @@ -43,11 +47,13 @@ class TraverseBench { var list: List[Int] = _ var vector: Vector[Int] = _ + var chain: Chain[Int] = _ @Setup def setup(): Unit = { list = 0.until(length).toList vector = 0.until(length).toVector + chain = Chain.fromSeq(0.until(length)) } @Benchmark @@ -83,6 +89,18 @@ class TraverseBench { } } + @Benchmark + def traverse_List(bh: Blackhole) = { + val result = listT.traverse_(list) { i => + Eval.later { + Blackhole.consumeCPU(Work) + i * 2 + } + } + + bh.consume(result.value) + } + @Benchmark def traverseFilterList(bh: Blackhole) = { val result = listTFilter.traverseFilter(list) { i => @@ -137,6 +155,18 @@ class TraverseBench { bh.consume(result.value) } + @Benchmark + def traverse_Vector(bh: Blackhole) = { + val result = vectorT.traverse_(vector) { i => + Eval.later { + Blackhole.consumeCPU(Work) + i * 2 + } + } + + bh.consume(result.value) + } + @Benchmark def traverseVectorError(bh: Blackhole) = { val result = vectorT.traverse(vector) { i => @@ -199,4 +229,61 @@ class TraverseBench { bh.consume(results) } + + @Benchmark + def traverseChain(bh: Blackhole) = { + val result = chainT.traverse(chain) { i => + Eval.later { + Blackhole.consumeCPU(Work) + i * 2 + } + } + + bh.consume(result.value) + } + + @Benchmark + def traverse_Chain(bh: Blackhole) = { + val result = chainT.traverse_(chain) { i => + Eval.later { + Blackhole.consumeCPU(Work) + i * 2 + } + } + + bh.consume(result.value) + } + + @Benchmark + def traverseChainError(bh: Blackhole) = { + val result = chainT.traverse(chain) { i => + Eval.later { + Blackhole.consumeCPU(Work) + + if (i == length * 0.3) { + throw Failure + } + + i * 2 + } + } + + try { + bh.consume(result.value) + } catch { + case Failure => () + } + } + + @Benchmark + def traverseFilterChain(bh: Blackhole) = { + val result = chainTFilter.traverseFilter(chain) { i => + Eval.later { + Blackhole.consumeCPU(Work) + if (i % 2 == 0) Some(i * 2) else None + } + } + + bh.consume(result.value) + } } From a1c9ff5d02f744cc6789627877c98059e6b4b3a5 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Fri, 25 Aug 2023 16:12:30 +0100 Subject: [PATCH 16/29] Use .void over .map(_ => ()) to give instances chance to optimize --- core/src/main/scala/cats/Traverse.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index 9109a860e1..fe1e62683f 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -310,11 +310,11 @@ object Traverse { val iter = fa.iterator if (iter.hasNext) { val first = iter.next() - G.map(iter.foldLeft(f(first)) { case (g, a) => + G.void(iter.foldLeft(f(first)) { case (g, a) => G.flatMap(g) { _ => f(a) } - })(_ => ()) + }) } else G.unit } From 8cc742e469f3718516cdb4fb5894aa3f29c737ef Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Wed, 8 Nov 2023 11:50:06 +0000 Subject: [PATCH 17/29] Experiment: use immutable data structures in optimized traverse Is this necessary? The monad laws should ensure that it's safe to use mutable builders. Nonetheless it will be good to confirm the performance delta for using immutable data structures --- .../scala-2.13+/cats/instances/arraySeq.scala | 4 ++-- core/src/main/scala/cats/Traverse.scala | 21 +++++++------------ core/src/main/scala/cats/TraverseFilter.scala | 21 +++++++------------ core/src/main/scala/cats/data/Chain.scala | 4 ++-- core/src/main/scala/cats/instances/list.scala | 4 ++-- .../src/main/scala/cats/instances/queue.scala | 8 ++++--- core/src/main/scala/cats/instances/seq.scala | 4 ++-- .../main/scala/cats/instances/vector.scala | 4 ++-- 8 files changed, 30 insertions(+), 40 deletions(-) diff --git a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala index 4ced92e1cb..d314e7b5eb 100644 --- a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala +++ b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala @@ -104,7 +104,7 @@ private[cats] object ArraySeqInstances { def traverse[G[_], A, B](fa: ArraySeq[A])(f: A => G[B])(implicit G: Applicative[G]): G[ArraySeq[B]] = G match { case x: StackSafeMonad[G] => - x.map(Traverse.traverseDirectly(Vector.newBuilder[B])(fa.iterator)(f)(x))(_.to(ArraySeq.untagged)) + x.map(Traverse.traverseDirectly(fa.iterator)(f)(x))(_.to(ArraySeq.untagged)) case _ => G.map(Chain.traverseViaChain(fa)(f))(_.iterator.to(ArraySeq.untagged)) @@ -233,7 +233,7 @@ private[cats] object ArraySeqInstances { )(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[ArraySeq[B]] = G match { case x: StackSafeMonad[G] => - x.map(TraverseFilter.traverseFilterDirectly(Vector.newBuilder[B])(fa.iterator)(f)(x))( + x.map(TraverseFilter.traverseFilterDirectly(fa.iterator)(f)(x))( _.to(ArraySeq.untagged) ) case _ => diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index fe1e62683f..a4378c4f1d 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -24,8 +24,6 @@ package cats import cats.data.State import cats.data.StateT import cats.kernel.compat.scalaVersionSpecific._ -import cats.StackSafeMonad -import scala.collection.mutable /** * Traverse, also known as Traversable. @@ -287,21 +285,16 @@ object Traverse { @deprecated("Use cats.syntax object imports", "2.2.0") object nonInheritedOps extends ToTraverseOps - private[cats] def traverseDirectly[Coll[x] <: IterableOnce[x], G[_], A, B]( - builder: mutable.Builder[B, Coll[B]] - )(fa: IterableOnce[A])(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Coll[B]] = { - val size = fa.knownSize - if (size >= 0) { - builder.sizeHint(size) - } - G.map(fa.iterator.foldLeft(G.pure(builder)) { case (accG, a) => + private[cats] def traverseDirectly[G[_], A, B]( + fa: IterableOnce[A] + )(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Vector[B]] = { + fa.iterator.foldLeft(G.pure(Vector.empty[B])) { case (accG, a) => G.flatMap(accG) { acc => - G.map(f(a)) { a => - acc += a - acc + G.map(f(a)) { b => + acc :+ b } } - })(_.result()) + } } private[cats] def traverse_Directly[G[_], A, B]( diff --git a/core/src/main/scala/cats/TraverseFilter.scala b/core/src/main/scala/cats/TraverseFilter.scala index 2cfa2145cb..6cd6714294 100644 --- a/core/src/main/scala/cats/TraverseFilter.scala +++ b/core/src/main/scala/cats/TraverseFilter.scala @@ -25,7 +25,6 @@ import cats.data.State import cats.kernel.compat.scalaVersionSpecific._ import scala.collection.immutable.{IntMap, TreeSet} -import scala.collection.mutable /** * `TraverseFilter`, also known as `Witherable`, represents list-like structures @@ -205,21 +204,17 @@ object TraverseFilter { @deprecated("Use cats.syntax object imports", "2.2.0") object nonInheritedOps extends ToTraverseFilterOps - private[cats] def traverseFilterDirectly[Coll[x] <: IterableOnce[x], G[_], A, B]( - builder: mutable.Builder[B, Coll[B]] - )(fa: IterableOnce[A])(f: A => G[Option[B]])(implicit G: StackSafeMonad[G]): G[Coll[B]] = { - val size = fa.knownSize - if (size >= 0) { - builder.sizeHint(size) - } - G.map(fa.iterator.foldLeft(G.pure(builder)) { case (bldrG, a) => - G.flatMap(bldrG) { bldr => + private[cats] def traverseFilterDirectly[G[_], A, B]( + fa: IterableOnce[A] + )(f: A => G[Option[B]])(implicit G: StackSafeMonad[G]): G[Vector[B]] = { + fa.iterator.foldLeft(G.pure(Vector.empty[B])) { case (bldrG, a) => + G.flatMap(bldrG) { acc => G.map(f(a)) { - case Some(b) => bldr += b - case None => bldr + case Some(b) => acc :+ b + case None => acc } } - })(_.result()) + } } } diff --git a/core/src/main/scala/cats/data/Chain.scala b/core/src/main/scala/cats/data/Chain.scala index 4fe4ec0cbe..1c55618aab 100644 --- a/core/src/main/scala/cats/data/Chain.scala +++ b/core/src/main/scala/cats/data/Chain.scala @@ -1243,7 +1243,7 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { else G match { case x: StackSafeMonad[G] => - x.map(Traverse.traverseDirectly(List.newBuilder[B])(fa.iterator)(f)(x))(Chain.fromSeq(_)) + x.map(Traverse.traverseDirectly(fa.iterator)(f)(x))(Chain.fromSeq(_)) case _ => traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() @@ -1372,7 +1372,7 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { else G match { case x: StackSafeMonad[G] => - G.map(TraverseFilter.traverseFilterDirectly(List.newBuilder[B])(fa.iterator)(f)(x))(Chain.fromSeq(_)) + G.map(TraverseFilter.traverseFilterDirectly(fa.iterator)(f)(x))(Chain.fromSeq(_)) case _ => traverseFilterViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/list.scala b/core/src/main/scala/cats/instances/list.scala index 2b18ee6810..eaeb6e298e 100644 --- a/core/src/main/scala/cats/instances/list.scala +++ b/core/src/main/scala/cats/instances/list.scala @@ -122,7 +122,7 @@ trait ListInstances extends cats.kernel.instances.ListInstances { if (fa.isEmpty) G.pure(Nil) else G match { - case x: StackSafeMonad[G] => Traverse.traverseDirectly[List, G, A, B](ListBuffer.empty[B])(fa)(f)(x) + case x: StackSafeMonad[G] => G.map(Traverse.traverseDirectly[G, A, B](fa)(f)(x))(_.toList) case _ => G.map(Chain.traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() @@ -320,7 +320,7 @@ private[instances] trait ListInstancesBinCompat0 { if (fa.isEmpty) G.pure(Nil) else G match { - case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(List.newBuilder[B])(fa)(f)(x) + case x: StackSafeMonad[G] => x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(_.toList) case _ => G.map(Chain.traverseFilterViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/queue.scala b/core/src/main/scala/cats/instances/queue.scala index 75aaf32a9d..628f6937b3 100644 --- a/core/src/main/scala/cats/instances/queue.scala +++ b/core/src/main/scala/cats/instances/queue.scala @@ -122,7 +122,8 @@ trait QueueInstances extends cats.kernel.instances.QueueInstances { if (fa.isEmpty) G.pure(Queue.empty[B]) else G match { - case x: StackSafeMonad[G] => Traverse.traverseDirectly(Queue.newBuilder[B])(fa)(f)(x) + case x: StackSafeMonad[G] => + G.map(Traverse.traverseDirectly(fa)(f)(x))(fromIterableOnce(_)) case _ => G.map(Chain.traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() @@ -222,7 +223,7 @@ trait QueueInstances extends cats.kernel.instances.QueueInstances { @suppressUnusedImportWarningForScalaVersionSpecific private object QueueInstances { private val catsStdTraverseFilterForQueue: TraverseFilter[Queue] = new TraverseFilter[Queue] { - val traverse: Traverse[Queue] = cats.instances.queue.catsStdInstancesForQueue + val traverse: Traverse[Queue] with Alternative[Queue] = cats.instances.queue.catsStdInstancesForQueue override def mapFilter[A, B](fa: Queue[A])(f: (A) => Option[B]): Queue[B] = fa.collect(Function.unlift(f)) @@ -239,7 +240,8 @@ private object QueueInstances { if (fa.isEmpty) G.pure(Queue.empty[B]) else G match { - case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(Queue.newBuilder[B])(fa)(f)(x) + case x: StackSafeMonad[G] => + x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(traverse.fromIterableOnce(_)) case _ => G.map(Chain.traverseFilterViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/seq.scala b/core/src/main/scala/cats/instances/seq.scala index 005139d6c2..f6ea076921 100644 --- a/core/src/main/scala/cats/instances/seq.scala +++ b/core/src/main/scala/cats/instances/seq.scala @@ -129,7 +129,7 @@ trait SeqInstances extends cats.kernel.instances.SeqInstances { final override def traverse[G[_], A, B](fa: Seq[A])(f: A => G[B])(implicit G: Applicative[G]): G[Seq[B]] = G match { case x: StackSafeMonad[G] => - Traverse.traverseDirectly(Seq.newBuilder[B])(fa)(f)(x) + x.map(Traverse.traverseDirectly(fa)(f)(x))(_.toSeq) case _ => G.map(Chain.traverseViaChain(fa.toIndexedSeq)(f))(_.toVector) } @@ -210,7 +210,7 @@ trait SeqInstances extends cats.kernel.instances.SeqInstances { def traverseFilter[G[_], A, B](fa: Seq[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Seq[B]] = G match { - case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(Seq.newBuilder[B])(fa)(f)(x) + case x: StackSafeMonad[G] => x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(_.toSeq) case _ => G.map(Chain.traverseFilterViaChain(fa.toIndexedSeq)(f))(_.toVector) } diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index d47d5f010a..9395374bda 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -126,7 +126,7 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances { final override def traverse[G[_], A, B](fa: Vector[A])(f: A => G[B])(implicit G: Applicative[G]): G[Vector[B]] = G match { - case x: StackSafeMonad[G] => Traverse.traverseDirectly(Vector.newBuilder[B])(fa)(f)(x) + case x: StackSafeMonad[G] => Traverse.traverseDirectly(fa)(f)(x) case _ => G.map(Chain.traverseViaChain(fa)(f))(_.toVector) } @@ -271,7 +271,7 @@ private[instances] trait VectorInstancesBinCompat0 { def traverseFilter[G[_], A, B](fa: Vector[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Vector[B]] = G match { - case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(Vector.newBuilder[B])(fa)(f)(x) + case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(fa)(f)(x) case _ => G.map(Chain.traverseFilterViaChain(fa)(f))(_.toVector) } From e3ae584735d444bff43cf7a012d11aeaff7f1237 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Fri, 10 Nov 2023 17:29:22 +0000 Subject: [PATCH 18/29] Experiment: use immutable List for optimized traverse To see if this is more performant than an immutable vector --- core/src/main/scala/cats/Traverse.scala | 8 ++++---- core/src/main/scala/cats/instances/list.scala | 2 +- core/src/main/scala/cats/instances/vector.scala | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index a4378c4f1d..7d4adc110b 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -287,14 +287,14 @@ object Traverse { private[cats] def traverseDirectly[G[_], A, B]( fa: IterableOnce[A] - )(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Vector[B]] = { - fa.iterator.foldLeft(G.pure(Vector.empty[B])) { case (accG, a) => + )(f: A => G[B])(implicit G: StackSafeMonad[G]): G[List[B]] = { + G.map(fa.iterator.foldLeft(G.pure(List.empty[B])) { case (accG, a) => G.flatMap(accG) { acc => G.map(f(a)) { b => - acc :+ b + b :: acc } } - } + })(_.reverse) } private[cats] def traverse_Directly[G[_], A, B]( diff --git a/core/src/main/scala/cats/instances/list.scala b/core/src/main/scala/cats/instances/list.scala index eaeb6e298e..7afda04293 100644 --- a/core/src/main/scala/cats/instances/list.scala +++ b/core/src/main/scala/cats/instances/list.scala @@ -122,7 +122,7 @@ trait ListInstances extends cats.kernel.instances.ListInstances { if (fa.isEmpty) G.pure(Nil) else G match { - case x: StackSafeMonad[G] => G.map(Traverse.traverseDirectly[G, A, B](fa)(f)(x))(_.toList) + case x: StackSafeMonad[G] => Traverse.traverseDirectly[G, A, B](fa)(f)(x) case _ => G.map(Chain.traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index 9395374bda..b927ede1f1 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -126,7 +126,7 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances { final override def traverse[G[_], A, B](fa: Vector[A])(f: A => G[B])(implicit G: Applicative[G]): G[Vector[B]] = G match { - case x: StackSafeMonad[G] => Traverse.traverseDirectly(fa)(f)(x) + case x: StackSafeMonad[G] => x.map(Traverse.traverseDirectly(fa)(f)(x))(_.toVector) case _ => G.map(Chain.traverseViaChain(fa)(f))(_.toVector) } From f95b0876d205f71559ef05d92cbcec8c1a700c40 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Tue, 14 Nov 2023 12:49:07 +0000 Subject: [PATCH 19/29] Use applicative methods instead of flatMap For the benefit of Monads with optimized Applicative operations eg parsers --- core/src/main/scala/cats/Traverse.scala | 10 +++------- core/src/main/scala/cats/TraverseFilter.scala | 8 +++----- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index 7d4adc110b..0031c7757f 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -289,10 +289,8 @@ object Traverse { fa: IterableOnce[A] )(f: A => G[B])(implicit G: StackSafeMonad[G]): G[List[B]] = { G.map(fa.iterator.foldLeft(G.pure(List.empty[B])) { case (accG, a) => - G.flatMap(accG) { acc => - G.map(f(a)) { b => - b :: acc - } + G.map2(accG, f(a)) { case (acc, x) => + x :: acc } })(_.reverse) } @@ -304,9 +302,7 @@ object Traverse { if (iter.hasNext) { val first = iter.next() G.void(iter.foldLeft(f(first)) { case (g, a) => - G.flatMap(g) { _ => - f(a) - } + G.productR(g)(f(a)) }) } else G.unit } diff --git a/core/src/main/scala/cats/TraverseFilter.scala b/core/src/main/scala/cats/TraverseFilter.scala index 6cd6714294..0f15703df8 100644 --- a/core/src/main/scala/cats/TraverseFilter.scala +++ b/core/src/main/scala/cats/TraverseFilter.scala @@ -208,11 +208,9 @@ object TraverseFilter { fa: IterableOnce[A] )(f: A => G[Option[B]])(implicit G: StackSafeMonad[G]): G[Vector[B]] = { fa.iterator.foldLeft(G.pure(Vector.empty[B])) { case (bldrG, a) => - G.flatMap(bldrG) { acc => - G.map(f(a)) { - case Some(b) => acc :+ b - case None => acc - } + G.map2(bldrG, f(a)) { + case (acc, Some(b)) => acc :+ b + case (acc, None) => acc } } } From c5d90a69657bd7a0cc0a654cfbcd798ed9f18274 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Fri, 8 Dec 2023 15:02:18 +0000 Subject: [PATCH 20/29] Experiment: use Chain for aggregating our optimized traverse --- core/src/main/scala-2.13+/cats/instances/arraySeq.scala | 2 +- core/src/main/scala/cats/Traverse.scala | 9 +++++---- core/src/main/scala/cats/data/Chain.scala | 2 +- core/src/main/scala/cats/instances/list.scala | 2 +- core/src/main/scala/cats/instances/queue.scala | 2 +- core/src/main/scala/cats/instances/seq.scala | 2 +- 6 files changed, 10 insertions(+), 9 deletions(-) diff --git a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala index d314e7b5eb..4bfc923b87 100644 --- a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala +++ b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala @@ -104,7 +104,7 @@ private[cats] object ArraySeqInstances { def traverse[G[_], A, B](fa: ArraySeq[A])(f: A => G[B])(implicit G: Applicative[G]): G[ArraySeq[B]] = G match { case x: StackSafeMonad[G] => - x.map(Traverse.traverseDirectly(fa.iterator)(f)(x))(_.to(ArraySeq.untagged)) + x.map(Traverse.traverseDirectly(fa.iterator)(f)(x))(_.iterator.to(ArraySeq.untagged)) case _ => G.map(Chain.traverseViaChain(fa)(f))(_.iterator.to(ArraySeq.untagged)) diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index 0031c7757f..cc2993ee69 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -21,6 +21,7 @@ package cats +import cats.data.Chain import cats.data.State import cats.data.StateT import cats.kernel.compat.scalaVersionSpecific._ @@ -287,12 +288,12 @@ object Traverse { private[cats] def traverseDirectly[G[_], A, B]( fa: IterableOnce[A] - )(f: A => G[B])(implicit G: StackSafeMonad[G]): G[List[B]] = { - G.map(fa.iterator.foldLeft(G.pure(List.empty[B])) { case (accG, a) => + )(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Chain[B]] = { + fa.iterator.foldLeft(G.pure(Chain.empty[B])) { case (accG, a) => G.map2(accG, f(a)) { case (acc, x) => - x :: acc + acc :+ x } - })(_.reverse) + } } private[cats] def traverse_Directly[G[_], A, B]( diff --git a/core/src/main/scala/cats/data/Chain.scala b/core/src/main/scala/cats/data/Chain.scala index 1c55618aab..28b32cd577 100644 --- a/core/src/main/scala/cats/data/Chain.scala +++ b/core/src/main/scala/cats/data/Chain.scala @@ -1243,7 +1243,7 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { else G match { case x: StackSafeMonad[G] => - x.map(Traverse.traverseDirectly(fa.iterator)(f)(x))(Chain.fromSeq(_)) + Traverse.traverseDirectly(fa.iterator)(f)(x) case _ => traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/list.scala b/core/src/main/scala/cats/instances/list.scala index 7afda04293..a1306f159b 100644 --- a/core/src/main/scala/cats/instances/list.scala +++ b/core/src/main/scala/cats/instances/list.scala @@ -122,7 +122,7 @@ trait ListInstances extends cats.kernel.instances.ListInstances { if (fa.isEmpty) G.pure(Nil) else G match { - case x: StackSafeMonad[G] => Traverse.traverseDirectly[G, A, B](fa)(f)(x) + case x: StackSafeMonad[G] => x.map(Traverse.traverseDirectly[G, A, B](fa)(f)(x))(_.toList) case _ => G.map(Chain.traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/queue.scala b/core/src/main/scala/cats/instances/queue.scala index 628f6937b3..3c7b2a9633 100644 --- a/core/src/main/scala/cats/instances/queue.scala +++ b/core/src/main/scala/cats/instances/queue.scala @@ -123,7 +123,7 @@ trait QueueInstances extends cats.kernel.instances.QueueInstances { else G match { case x: StackSafeMonad[G] => - G.map(Traverse.traverseDirectly(fa)(f)(x))(fromIterableOnce(_)) + G.map(Traverse.traverseDirectly(fa)(f)(x))(c => fromIterableOnce(c.iterator)) case _ => G.map(Chain.traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/seq.scala b/core/src/main/scala/cats/instances/seq.scala index f6ea076921..18ba3f8688 100644 --- a/core/src/main/scala/cats/instances/seq.scala +++ b/core/src/main/scala/cats/instances/seq.scala @@ -129,7 +129,7 @@ trait SeqInstances extends cats.kernel.instances.SeqInstances { final override def traverse[G[_], A, B](fa: Seq[A])(f: A => G[B])(implicit G: Applicative[G]): G[Seq[B]] = G match { case x: StackSafeMonad[G] => - x.map(Traverse.traverseDirectly(fa)(f)(x))(_.toSeq) + x.map(Traverse.traverseDirectly(fa)(f)(x))(_.toList) case _ => G.map(Chain.traverseViaChain(fa.toIndexedSeq)(f))(_.toVector) } From 2d5f4d7917804e921488f5f2e263e8349a6ceb42 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Wed, 13 Dec 2023 15:58:20 +0000 Subject: [PATCH 21/29] Implement optimized traverseFilter in terms of Chain as well --- core/src/main/scala-2.13+/cats/instances/arraySeq.scala | 2 +- core/src/main/scala/cats/TraverseFilter.scala | 6 +++--- core/src/main/scala/cats/data/Chain.scala | 2 +- core/src/main/scala/cats/instances/queue.scala | 2 +- core/src/main/scala/cats/instances/seq.scala | 2 +- core/src/main/scala/cats/instances/vector.scala | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala index 4bfc923b87..cc254fbcae 100644 --- a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala +++ b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala @@ -234,7 +234,7 @@ private[cats] object ArraySeqInstances { G match { case x: StackSafeMonad[G] => x.map(TraverseFilter.traverseFilterDirectly(fa.iterator)(f)(x))( - _.to(ArraySeq.untagged) + _.iterator.to(ArraySeq.untagged) ) case _ => fa.foldRight(Eval.now(G.pure(ArraySeq.untagged.empty[B]))) { case (x, xse) => diff --git a/core/src/main/scala/cats/TraverseFilter.scala b/core/src/main/scala/cats/TraverseFilter.scala index 0f15703df8..0907ef3be7 100644 --- a/core/src/main/scala/cats/TraverseFilter.scala +++ b/core/src/main/scala/cats/TraverseFilter.scala @@ -21,7 +21,7 @@ package cats -import cats.data.State +import cats.data.{Chain, State} import cats.kernel.compat.scalaVersionSpecific._ import scala.collection.immutable.{IntMap, TreeSet} @@ -206,8 +206,8 @@ object TraverseFilter { private[cats] def traverseFilterDirectly[G[_], A, B]( fa: IterableOnce[A] - )(f: A => G[Option[B]])(implicit G: StackSafeMonad[G]): G[Vector[B]] = { - fa.iterator.foldLeft(G.pure(Vector.empty[B])) { case (bldrG, a) => + )(f: A => G[Option[B]])(implicit G: StackSafeMonad[G]): G[Chain[B]] = { + fa.iterator.foldLeft(G.pure(Chain.empty[B])) { case (bldrG, a) => G.map2(bldrG, f(a)) { case (acc, Some(b)) => acc :+ b case (acc, None) => acc diff --git a/core/src/main/scala/cats/data/Chain.scala b/core/src/main/scala/cats/data/Chain.scala index 28b32cd577..e113cae684 100644 --- a/core/src/main/scala/cats/data/Chain.scala +++ b/core/src/main/scala/cats/data/Chain.scala @@ -1372,7 +1372,7 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { else G match { case x: StackSafeMonad[G] => - G.map(TraverseFilter.traverseFilterDirectly(fa.iterator)(f)(x))(Chain.fromSeq(_)) + TraverseFilter.traverseFilterDirectly(fa.iterator)(f)(x) case _ => traverseFilterViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/queue.scala b/core/src/main/scala/cats/instances/queue.scala index 3c7b2a9633..05bd98f8b7 100644 --- a/core/src/main/scala/cats/instances/queue.scala +++ b/core/src/main/scala/cats/instances/queue.scala @@ -241,7 +241,7 @@ private object QueueInstances { else G match { case x: StackSafeMonad[G] => - x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(traverse.fromIterableOnce(_)) + x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(c => traverse.fromIterableOnce(c.iterator)) case _ => G.map(Chain.traverseFilterViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/seq.scala b/core/src/main/scala/cats/instances/seq.scala index 18ba3f8688..0ad0005575 100644 --- a/core/src/main/scala/cats/instances/seq.scala +++ b/core/src/main/scala/cats/instances/seq.scala @@ -210,7 +210,7 @@ trait SeqInstances extends cats.kernel.instances.SeqInstances { def traverseFilter[G[_], A, B](fa: Seq[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Seq[B]] = G match { - case x: StackSafeMonad[G] => x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(_.toSeq) + case x: StackSafeMonad[G] => x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(_.toVector) case _ => G.map(Chain.traverseFilterViaChain(fa.toIndexedSeq)(f))(_.toVector) } diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index b927ede1f1..06f93b4fef 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -271,7 +271,7 @@ private[instances] trait VectorInstancesBinCompat0 { def traverseFilter[G[_], A, B](fa: Vector[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Vector[B]] = G match { - case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(fa)(f)(x) + case x: StackSafeMonad[G] => x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(_.toVector) case _ => G.map(Chain.traverseFilterViaChain(fa)(f))(_.toVector) } From 702ab8b9d1ebad207bb7c97eb671da333e667ea0 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Tue, 19 Dec 2023 11:50:56 +0000 Subject: [PATCH 22/29] Vector-based optimized traverse and traverseFilter in the same commit for the sake of benchmarking --- core/src/main/scala/cats/Traverse.scala | 4 ++-- core/src/main/scala/cats/TraverseFilter.scala | 4 ++-- core/src/main/scala/cats/data/Chain.scala | 6 +++--- core/src/main/scala/cats/instances/vector.scala | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index cc2993ee69..f1856fdc53 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -288,8 +288,8 @@ object Traverse { private[cats] def traverseDirectly[G[_], A, B]( fa: IterableOnce[A] - )(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Chain[B]] = { - fa.iterator.foldLeft(G.pure(Chain.empty[B])) { case (accG, a) => + )(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Vector[B]] = { + fa.iterator.foldLeft(G.pure(Vector.empty[B])) { case (accG, a) => G.map2(accG, f(a)) { case (acc, x) => acc :+ x } diff --git a/core/src/main/scala/cats/TraverseFilter.scala b/core/src/main/scala/cats/TraverseFilter.scala index 0907ef3be7..25521b35f2 100644 --- a/core/src/main/scala/cats/TraverseFilter.scala +++ b/core/src/main/scala/cats/TraverseFilter.scala @@ -206,8 +206,8 @@ object TraverseFilter { private[cats] def traverseFilterDirectly[G[_], A, B]( fa: IterableOnce[A] - )(f: A => G[Option[B]])(implicit G: StackSafeMonad[G]): G[Chain[B]] = { - fa.iterator.foldLeft(G.pure(Chain.empty[B])) { case (bldrG, a) => + )(f: A => G[Option[B]])(implicit G: StackSafeMonad[G]): G[Vector[B]] = { + fa.iterator.foldLeft(G.pure(Vector.empty[B])) { case (bldrG, a) => G.map2(bldrG, f(a)) { case (acc, Some(b)) => acc :+ b case (acc, None) => acc diff --git a/core/src/main/scala/cats/data/Chain.scala b/core/src/main/scala/cats/data/Chain.scala index e113cae684..721cc66a50 100644 --- a/core/src/main/scala/cats/data/Chain.scala +++ b/core/src/main/scala/cats/data/Chain.scala @@ -1243,7 +1243,7 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { else G match { case x: StackSafeMonad[G] => - Traverse.traverseDirectly(fa.iterator)(f)(x) + x.map(Traverse.traverseDirectly(fa.iterator)(f)(x))(fromIterableOnce(_)) case _ => traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() @@ -1355,7 +1355,7 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { } implicit val catsDataTraverseFilterForChain: TraverseFilter[Chain] = new TraverseFilter[Chain] { - def traverse: Traverse[Chain] = Chain.catsDataInstancesForChain + def traverse: Traverse[Chain] with Alternative[Chain] = Chain.catsDataInstancesForChain override def filter[A](fa: Chain[A])(f: A => Boolean): Chain[A] = fa.filter(f) @@ -1372,7 +1372,7 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { else G match { case x: StackSafeMonad[G] => - TraverseFilter.traverseFilterDirectly(fa.iterator)(f)(x) + x.map(TraverseFilter.traverseFilterDirectly(fa.iterator)(f)(x))(traverse.fromIterableOnce(_)) case _ => traverseFilterViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index 06f93b4fef..b927ede1f1 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -271,7 +271,7 @@ private[instances] trait VectorInstancesBinCompat0 { def traverseFilter[G[_], A, B](fa: Vector[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Vector[B]] = G match { - case x: StackSafeMonad[G] => x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(_.toVector) + case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(fa)(f)(x) case _ => G.map(Chain.traverseFilterViaChain(fa)(f))(_.toVector) } From b08196e8526f6025d009ac0c8efc0a0e0ebcb1c4 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Fri, 22 Dec 2023 14:22:56 +0000 Subject: [PATCH 23/29] Retroactive attempt to establish a baseline including the new benchmarks --- core/src/main/scala/cats/data/Chain.scala | 5 ----- core/src/main/scala/cats/instances/list.scala | 6 +----- core/src/main/scala/cats/instances/vector.scala | 7 ++----- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/core/src/main/scala/cats/data/Chain.scala b/core/src/main/scala/cats/data/Chain.scala index 721cc66a50..7ba5e40d54 100644 --- a/core/src/main/scala/cats/data/Chain.scala +++ b/core/src/main/scala/cats/data/Chain.scala @@ -1242,8 +1242,6 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { if (fa.isEmpty) G.pure(Chain.nil) else G match { - case x: StackSafeMonad[G] => - x.map(Traverse.traverseDirectly(fa.iterator)(f)(x))(fromIterableOnce(_)) case _ => traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() @@ -1254,7 +1252,6 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { override def traverse_[G[_], A, B](fa: Chain[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = G match { - case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa.iterator)(f)(x) case _ => foldRight(fa, Always(G.pure(()))) { (a, acc) => G.map2Eval(f(a), acc) { (_, _) => @@ -1371,8 +1368,6 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { if (fa.isEmpty) G.pure(Chain.nil) else G match { - case x: StackSafeMonad[G] => - x.map(TraverseFilter.traverseFilterDirectly(fa.iterator)(f)(x))(traverse.fromIterableOnce(_)) case _ => traverseFilterViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/list.scala b/core/src/main/scala/cats/instances/list.scala index a1306f159b..ecd37c9b0d 100644 --- a/core/src/main/scala/cats/instances/list.scala +++ b/core/src/main/scala/cats/instances/list.scala @@ -23,7 +23,6 @@ package cats package instances import cats.data.{Chain, Ior, ZipList} -import cats.StackSafeMonad import cats.instances.StaticMethods.appendAll import cats.kernel.compat.scalaVersionSpecific._ import cats.kernel.instances.StaticMethods.wrapMutableIndexedSeq @@ -122,7 +121,6 @@ trait ListInstances extends cats.kernel.instances.ListInstances { if (fa.isEmpty) G.pure(Nil) else G match { - case x: StackSafeMonad[G] => x.map(Traverse.traverseDirectly[G, A, B](fa)(f)(x))(_.toList) case _ => G.map(Chain.traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() @@ -136,8 +134,7 @@ trait ListInstances extends cats.kernel.instances.ListInstances { */ override def traverse_[G[_], A, B](fa: List[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = { G match { - case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) - case _ => + case _ => // the cost of this is O(size log size) // c(n) = n + 2 * c(n/2) = n + 2(n/2 log (n/2)) = n + n (logn - 1) = n log n // invariant: size >= 1 @@ -320,7 +317,6 @@ private[instances] trait ListInstancesBinCompat0 { if (fa.isEmpty) G.pure(Nil) else G match { - case x: StackSafeMonad[G] => x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(_.toList) case _ => G.map(Chain.traverseFilterViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index b927ede1f1..a47a2f0142 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -126,8 +126,7 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances { final override def traverse[G[_], A, B](fa: Vector[A])(f: A => G[B])(implicit G: Applicative[G]): G[Vector[B]] = G match { - case x: StackSafeMonad[G] => x.map(Traverse.traverseDirectly(fa)(f)(x))(_.toVector) - case _ => G.map(Chain.traverseViaChain(fa)(f))(_.toVector) + case _ => G.map(Chain.traverseViaChain(fa)(f))(_.toVector) } final override def updated_[A, B >: A](fa: Vector[A], idx: Long, b: B): Option[Vector[B]] = @@ -142,8 +141,7 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances { */ override def traverse_[G[_], A, B](fa: Vector[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = { G match { - case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) - case _ => + case _ => // the cost of this is O(size) // c(n) = 1 + 2 * c(n/2) // invariant: size >= 1 @@ -271,7 +269,6 @@ private[instances] trait VectorInstancesBinCompat0 { def traverseFilter[G[_], A, B](fa: Vector[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Vector[B]] = G match { - case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(fa)(f)(x) case _ => G.map(Chain.traverseFilterViaChain(fa)(f))(_.toVector) } From 92c91e438cb6479381513776399ae554a53e6f78 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Mon, 4 Mar 2024 14:37:32 +0000 Subject: [PATCH 24/29] Revert "Retroactive attempt to establish a baseline including the new benchmarks" This reverts commit b08196e8526f6025d009ac0c8efc0a0e0ebcb1c4. We've got our baseline benchmarks now --- core/src/main/scala/cats/data/Chain.scala | 5 +++++ core/src/main/scala/cats/instances/list.scala | 6 +++++- core/src/main/scala/cats/instances/vector.scala | 7 +++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/core/src/main/scala/cats/data/Chain.scala b/core/src/main/scala/cats/data/Chain.scala index 7ba5e40d54..721cc66a50 100644 --- a/core/src/main/scala/cats/data/Chain.scala +++ b/core/src/main/scala/cats/data/Chain.scala @@ -1242,6 +1242,8 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { if (fa.isEmpty) G.pure(Chain.nil) else G match { + case x: StackSafeMonad[G] => + x.map(Traverse.traverseDirectly(fa.iterator)(f)(x))(fromIterableOnce(_)) case _ => traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() @@ -1252,6 +1254,7 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { override def traverse_[G[_], A, B](fa: Chain[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = G match { + case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa.iterator)(f)(x) case _ => foldRight(fa, Always(G.pure(()))) { (a, acc) => G.map2Eval(f(a), acc) { (_, _) => @@ -1368,6 +1371,8 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { if (fa.isEmpty) G.pure(Chain.nil) else G match { + case x: StackSafeMonad[G] => + x.map(TraverseFilter.traverseFilterDirectly(fa.iterator)(f)(x))(traverse.fromIterableOnce(_)) case _ => traverseFilterViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/list.scala b/core/src/main/scala/cats/instances/list.scala index ecd37c9b0d..a1306f159b 100644 --- a/core/src/main/scala/cats/instances/list.scala +++ b/core/src/main/scala/cats/instances/list.scala @@ -23,6 +23,7 @@ package cats package instances import cats.data.{Chain, Ior, ZipList} +import cats.StackSafeMonad import cats.instances.StaticMethods.appendAll import cats.kernel.compat.scalaVersionSpecific._ import cats.kernel.instances.StaticMethods.wrapMutableIndexedSeq @@ -121,6 +122,7 @@ trait ListInstances extends cats.kernel.instances.ListInstances { if (fa.isEmpty) G.pure(Nil) else G match { + case x: StackSafeMonad[G] => x.map(Traverse.traverseDirectly[G, A, B](fa)(f)(x))(_.toList) case _ => G.map(Chain.traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() @@ -134,7 +136,8 @@ trait ListInstances extends cats.kernel.instances.ListInstances { */ override def traverse_[G[_], A, B](fa: List[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = { G match { - case _ => + case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) + case _ => // the cost of this is O(size log size) // c(n) = n + 2 * c(n/2) = n + 2(n/2 log (n/2)) = n + n (logn - 1) = n log n // invariant: size >= 1 @@ -317,6 +320,7 @@ private[instances] trait ListInstancesBinCompat0 { if (fa.isEmpty) G.pure(Nil) else G match { + case x: StackSafeMonad[G] => x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(_.toList) case _ => G.map(Chain.traverseFilterViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index a47a2f0142..b927ede1f1 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -126,7 +126,8 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances { final override def traverse[G[_], A, B](fa: Vector[A])(f: A => G[B])(implicit G: Applicative[G]): G[Vector[B]] = G match { - case _ => G.map(Chain.traverseViaChain(fa)(f))(_.toVector) + case x: StackSafeMonad[G] => x.map(Traverse.traverseDirectly(fa)(f)(x))(_.toVector) + case _ => G.map(Chain.traverseViaChain(fa)(f))(_.toVector) } final override def updated_[A, B >: A](fa: Vector[A], idx: Long, b: B): Option[Vector[B]] = @@ -141,7 +142,8 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances { */ override def traverse_[G[_], A, B](fa: Vector[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = { G match { - case _ => + case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) + case _ => // the cost of this is O(size) // c(n) = 1 + 2 * c(n/2) // invariant: size >= 1 @@ -269,6 +271,7 @@ private[instances] trait VectorInstancesBinCompat0 { def traverseFilter[G[_], A, B](fa: Vector[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Vector[B]] = G match { + case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(fa)(f)(x) case _ => G.map(Chain.traverseFilterViaChain(fa)(f))(_.toVector) } From eebed5bd914b7be44d64864d75d7905483672070 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Mon, 4 Mar 2024 22:23:18 +0000 Subject: [PATCH 25/29] Use Applicative#unit to limit allocations --- core/src/main/scala/cats/data/Chain.scala | 2 +- core/src/main/scala/cats/instances/queue.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/cats/data/Chain.scala b/core/src/main/scala/cats/data/Chain.scala index 838725713e..43215e0fd2 100644 --- a/core/src/main/scala/cats/data/Chain.scala +++ b/core/src/main/scala/cats/data/Chain.scala @@ -1258,7 +1258,7 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { G match { case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa.iterator)(f)(x) case _ => - foldRight(fa, Always(G.pure(()))) { (a, acc) => + foldRight(fa, Eval.now(G.unit)) { (a, acc) => G.map2Eval(f(a), acc) { (_, _) => () } diff --git a/core/src/main/scala/cats/instances/queue.scala b/core/src/main/scala/cats/instances/queue.scala index 05bd98f8b7..bb10850423 100644 --- a/core/src/main/scala/cats/instances/queue.scala +++ b/core/src/main/scala/cats/instances/queue.scala @@ -138,7 +138,7 @@ trait QueueInstances extends cats.kernel.instances.QueueInstances { G match { case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) case _ => - foldRight(fa, Always(G.pure(()))) { (a, acc) => + foldRight(fa, Eval.now(G.unit)) { (a, acc) => G.map2Eval(f(a), acc) { (_, _) => () } From 803107de2f728f648800513ef5cff242a1172fbf Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Tue, 5 Mar 2024 15:25:40 +0000 Subject: [PATCH 26/29] More Applicative#unit to limit allocations --- core/src/main/scala-2.13+/cats/instances/arraySeq.scala | 2 +- core/src/main/scala/cats/instances/seq.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala index cc254fbcae..f41a781ad6 100644 --- a/core/src/main/scala-2.13+/cats/instances/arraySeq.scala +++ b/core/src/main/scala-2.13+/cats/instances/arraySeq.scala @@ -114,7 +114,7 @@ private[cats] object ArraySeqInstances { G match { case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) case _ => - foldRight(fa, Always(G.pure(()))) { (a, acc) => + foldRight(fa, Eval.now(G.unit)) { (a, acc) => G.map2Eval(f(a), acc) { (_, _) => () } diff --git a/core/src/main/scala/cats/instances/seq.scala b/core/src/main/scala/cats/instances/seq.scala index 0ad0005575..f4d0c5a5a7 100644 --- a/core/src/main/scala/cats/instances/seq.scala +++ b/core/src/main/scala/cats/instances/seq.scala @@ -138,7 +138,7 @@ trait SeqInstances extends cats.kernel.instances.SeqInstances { G match { case x: StackSafeMonad[G] => Traverse.traverse_Directly(fa)(f)(x) case _ => - foldRight(fa, Always(G.pure(()))) { (a, acc) => + foldRight(fa, Eval.now(G.unit)) { (a, acc) => G.map2Eval(f(a), acc) { (_, _) => () } From 0ee49e6f678d51739f4e66532a5ecbc68a43f619 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Thu, 7 Mar 2024 17:14:17 +0000 Subject: [PATCH 27/29] Remove mutable builder from Map traverse as its unlawful --- core/src/main/scala/cats/instances/map.scala | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/core/src/main/scala/cats/instances/map.scala b/core/src/main/scala/cats/instances/map.scala index 433d06aedf..c469f88b5d 100644 --- a/core/src/main/scala/cats/instances/map.scala +++ b/core/src/main/scala/cats/instances/map.scala @@ -46,14 +46,9 @@ trait MapInstances extends cats.kernel.instances.MapInstances { else G match { case x: StackSafeMonad[G] => - x.map(fa.foldLeft(G.pure(Map.newBuilder[K, B])) { case (accG, (k, a)) => - x.flatMap(accG) { acc => - G.map(f(a)) { a => - acc += k -> a - acc - } - } - })(_.result()) + fa.iterator.foldLeft(G.pure(Map.empty[K, B])) { case (accG, (k, a)) => + x.map2(accG, f(a)) { case (acc, b) => acc + (k -> b) } + } case _ => G.map(Chain.traverseViaChain(fa.toIndexedSeq) { case (k, a) => G.map(f(a))((k, _)) From 6529be7bd4d3547fb3b0b1390f13905a9f357415 Mon Sep 17 00:00:00 2001 From: Andrew Valencik Date: Fri, 22 Mar 2024 09:28:09 -0400 Subject: [PATCH 28/29] Use Chain in `traverseDirectly` --- core/src/main/scala/cats/Traverse.scala | 4 ++-- core/src/main/scala/cats/TraverseFilter.scala | 4 ++-- core/src/main/scala/cats/data/Chain.scala | 4 ++-- core/src/main/scala/cats/instances/vector.scala | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/core/src/main/scala/cats/Traverse.scala b/core/src/main/scala/cats/Traverse.scala index f1856fdc53..cc2993ee69 100644 --- a/core/src/main/scala/cats/Traverse.scala +++ b/core/src/main/scala/cats/Traverse.scala @@ -288,8 +288,8 @@ object Traverse { private[cats] def traverseDirectly[G[_], A, B]( fa: IterableOnce[A] - )(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Vector[B]] = { - fa.iterator.foldLeft(G.pure(Vector.empty[B])) { case (accG, a) => + )(f: A => G[B])(implicit G: StackSafeMonad[G]): G[Chain[B]] = { + fa.iterator.foldLeft(G.pure(Chain.empty[B])) { case (accG, a) => G.map2(accG, f(a)) { case (acc, x) => acc :+ x } diff --git a/core/src/main/scala/cats/TraverseFilter.scala b/core/src/main/scala/cats/TraverseFilter.scala index 25521b35f2..0907ef3be7 100644 --- a/core/src/main/scala/cats/TraverseFilter.scala +++ b/core/src/main/scala/cats/TraverseFilter.scala @@ -206,8 +206,8 @@ object TraverseFilter { private[cats] def traverseFilterDirectly[G[_], A, B]( fa: IterableOnce[A] - )(f: A => G[Option[B]])(implicit G: StackSafeMonad[G]): G[Vector[B]] = { - fa.iterator.foldLeft(G.pure(Vector.empty[B])) { case (bldrG, a) => + )(f: A => G[Option[B]])(implicit G: StackSafeMonad[G]): G[Chain[B]] = { + fa.iterator.foldLeft(G.pure(Chain.empty[B])) { case (bldrG, a) => G.map2(bldrG, f(a)) { case (acc, Some(b)) => acc :+ b case (acc, None) => acc diff --git a/core/src/main/scala/cats/data/Chain.scala b/core/src/main/scala/cats/data/Chain.scala index 43215e0fd2..2248a562ea 100644 --- a/core/src/main/scala/cats/data/Chain.scala +++ b/core/src/main/scala/cats/data/Chain.scala @@ -1245,7 +1245,7 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { else G match { case x: StackSafeMonad[G] => - x.map(Traverse.traverseDirectly(fa.iterator)(f)(x))(fromIterableOnce(_)) + Traverse.traverseDirectly(fa.iterator)(f)(x) case _ => traverseViaChain { val as = collection.mutable.ArrayBuffer[A]() @@ -1374,7 +1374,7 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 { else G match { case x: StackSafeMonad[G] => - x.map(TraverseFilter.traverseFilterDirectly(fa.iterator)(f)(x))(traverse.fromIterableOnce(_)) + TraverseFilter.traverseFilterDirectly(fa.iterator)(f)(x) case _ => traverseFilterViaChain { val as = collection.mutable.ArrayBuffer[A]() diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index b927ede1f1..06f93b4fef 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -271,7 +271,7 @@ private[instances] trait VectorInstancesBinCompat0 { def traverseFilter[G[_], A, B](fa: Vector[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Vector[B]] = G match { - case x: StackSafeMonad[G] => TraverseFilter.traverseFilterDirectly(fa)(f)(x) + case x: StackSafeMonad[G] => x.map(TraverseFilter.traverseFilterDirectly(fa)(f)(x))(_.toVector) case _ => G.map(Chain.traverseFilterViaChain(fa)(f))(_.toVector) } From a62ce4c55f316c575e1f30c9fbe0aacda0a3252f Mon Sep 17 00:00:00 2001 From: Andrew Valencik Date: Fri, 22 Mar 2024 09:28:24 -0400 Subject: [PATCH 29/29] Use `toList` in seq traverse, not vector --- core/src/main/scala/cats/instances/seq.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/cats/instances/seq.scala b/core/src/main/scala/cats/instances/seq.scala index f4d0c5a5a7..32272eb1f9 100644 --- a/core/src/main/scala/cats/instances/seq.scala +++ b/core/src/main/scala/cats/instances/seq.scala @@ -131,7 +131,7 @@ trait SeqInstances extends cats.kernel.instances.SeqInstances { case x: StackSafeMonad[G] => x.map(Traverse.traverseDirectly(fa)(f)(x))(_.toList) case _ => - G.map(Chain.traverseViaChain(fa.toIndexedSeq)(f))(_.toVector) + G.map(Chain.traverseViaChain(fa.toIndexedSeq)(f))(_.toList) } override def traverse_[G[_], A, B](fa: Seq[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] =