Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
9ca5eec
Added benchmarks to reproduce issue
djspiewak Jul 10, 2025
d609d3e
Reimplemented parTraverseN to be much more efficient and less patholo…
djspiewak Jul 10, 2025
9f0b328
Fixed surfacing of inner errors/cancelation in parTraverseN_
djspiewak Jul 10, 2025
607ab9a
Swap to deferred to avoid orphaned errors
djspiewak Jul 11, 2025
569ce54
Ported over @durban's tests and fixed
djspiewak Jul 12, 2025
0109cc3
Added early abort when stop case is encountered
djspiewak Jul 22, 2025
a3f8fe8
Organized imports
djspiewak Jul 23, 2025
6a2588c
Fixed null test for Scala 3
djspiewak Jul 23, 2025
599b790
Removed spurious test that triggered scala.js bugs
djspiewak Jul 23, 2025
2e276bb
Added test for error short-circuiting
djspiewak Jul 23, 2025
3649a5d
Restructured failure case propagation
djspiewak Jul 27, 2025
584ce3b
Added more tests and shuffled preemption paths to actually, you know,…
djspiewak Mar 8, 2026
4c2e0d7
We need a bit more time in CI
djspiewak Mar 8, 2026
7709fd1
Moved short circuiting tests to JVM platform
djspiewak Mar 8, 2026
4aa9ae7
Scalafix
djspiewak Mar 8, 2026
417d359
Updated scaladoc and fixed a laziness bug
djspiewak Mar 8, 2026
e195227
Fixed function suspension in fiber
djspiewak Mar 8, 2026
6f90794
Fix typo
durban Mar 11, 2026
69e4abf
Add failing tests
durban Mar 11, 2026
2848998
scalafmt
durban Mar 11, 2026
288273d
Properly cancel all fibers after self-cancelation in `parTraverseN_`
djspiewak Mar 14, 2026
229ae07
Ensure everything is canceled in the preemption case of parTraverseN_
djspiewak Jun 22, 2026
a127f71
Fixed finalizer backpressure in error cases of parTraverseN
djspiewak Jun 22, 2026
7941542
Added race condition test on error interruption
djspiewak Jun 23, 2026
e3b4220
Scalafmt
djspiewak Jun 23, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@
package cats.effect.benchmarks

import cats.effect.IO
import cats.effect.syntax.all._
import cats.effect.unsafe.implicits.global
import cats.implicits.{catsSyntaxParallelTraverse1, toTraverseOps}

import org.openjdk.jmh.annotations._
import org.openjdk.jmh.infra.Blackhole

import scala.concurrent.duration._

import java.util.concurrent.TimeUnit

/**
Expand Down Expand Up @@ -55,6 +58,24 @@ class ParallelBenchmark {
def parTraverse(): Unit =
1.to(size).toList.parTraverse(_ => IO(Blackhole.consumeCPU(cpuTokens))).void.unsafeRunSync()

@Benchmark
def parTraverseN(): Unit =
1.to(size)
.toList
.parTraverseN(size / 100)(_ => IO(Blackhole.consumeCPU(cpuTokens)))
.void
.unsafeRunSync()

@Benchmark
def parTraverseNCancel(): Unit = {
val e = new RuntimeException
val test = 1.to(size * 100).toList.parTraverseN(size / 100) { _ =>
IO.sleep(100.millis) *> IO.raiseError(e)
}

test.attempt.void.unsafeRunSync()
}

@Benchmark
def traverse(): Unit =
1.to(size).toList.traverse(_ => IO(Blackhole.consumeCPU(cpuTokens))).void.unsafeRunSync()
Expand Down
183 changes: 175 additions & 8 deletions kernel/shared/src/main/scala/cats/effect/kernel/GenConcurrent.scala
Original file line number Diff line number Diff line change
Expand Up @@ -130,29 +130,196 @@ trait GenConcurrent[F[_], E] extends GenSpawn[F, E] {
parTraverseN_(n)(tma)(identity)

/**
* Like `Parallel.parTraverse`, but limits the degree of parallelism. Note that the semantics
* of this operation aim to maximise fairness: when a spot to execute becomes available, every
* task has a chance to claim it, and not only the next `n` tasks in `ta`
* Like `Parallel.parTraverse`, but limits the degree of parallelism. The semantics of this
* function are ordered based on the `Traverse`. The first ''n'' actions will be started
* first, with subsequent actions starting in order as each one completes. Actions which are
* reached earlier in `traverse` order will be started slightly sooner than later actions, in
* a non-blocking fashion. Any errors or self-cancelation will immediately abort the sequence.
* If multiple actions produce errors simultaneously, one of them will be nondeterministically
* selected for production. If all actions succeed, their results are returned in the same
* order as their corresponding inputs, regardless of the order in which they executed.
Comment thread
djspiewak marked this conversation as resolved.
*
* The `f` function is run as part of running the action: in parallel and subject to the
* limit.
*/
def parTraverseN[T[_]: Traverse, A, B](n: Int)(ta: T[A])(f: A => F[B]): F[T[B]] = {
require(n >= 1, s"Concurrency limit should be at least 1, was: $n")

Comment thread
djspiewak marked this conversation as resolved.
implicit val F: GenConcurrent[F, E] = this

MiniSemaphore[F](n).flatMap { sem => ta.parTraverse { a => sem.withPermit(f(a)) } }
F.deferred[Option[E]] flatMap { preempt =>
F.ref[Set[(Fiber[F, ?, ?], Deferred[F, Outcome[F, E, B]])]](Set()) flatMap {
supervision =>
// has to be done in parallel to avoid head of line issues
def cancelAll(cause: Option[E]) = supervision.get flatMap { states =>
val causeOC: Outcome[F, E, B] = cause match {
case Some(e) => Outcome.Errored(e)
case None => Outcome.Canceled()
}

states.toList parTraverse_ {
case (fiber, result) =>
result.complete(causeOC) *> fiber.cancel
}
}

MiniSemaphore[F](n) flatMap { sem =>
val results = ta traverse { a =>
preempt.tryGet flatMap {
case Some(Some(e)) => F.pure(F.raiseError[B](e))
case Some(None) => F.pure(F.canceled *> F.never[B])

case None =>
F.uncancelable { poll =>
F.deferred[Outcome[F, E, B]] flatMap { result =>
// acquire the semaphore *before* creating and starting the fiber
// the semaphore gates the traverse, and thus the spawning, not the execution
// the laziness is a poor mans defer; this ensures the f gets pushed to the fiber
val action = poll(sem.acquire) *> (F.unit >> f(a))
Comment thread
djspiewak marked this conversation as resolved.
.guaranteeCase { oc =>
val completion = oc match {
case Outcome.Succeeded(_) =>
preempt.tryGet flatMap {
case Some(Some(e)) =>
result.complete(Outcome.Errored(e))

case Some(None) =>
result.complete(Outcome.Canceled())

case None =>
result.complete(oc)
}

case Outcome.Errored(e) =>
preempt
.complete(Some(e))
.ifM(
// we can't fire-and-forget this one because final results don't block on cancelation
result.complete(oc) <* cancelAll(Some(e)),
false.pure[F])

case Outcome.Canceled() =>
preempt
.complete(None)
.ifM(
// we *need* to fire-and-forget this cancelation to avoid deadlock loops when we're already canceling
// we won't return prematurely because we have a final `onCancel` on the results sequence
result.complete(oc) <* cancelAll(None).start,
Comment thread
djspiewak marked this conversation as resolved.
false.pure[F]
)
}

completion *> sem.release
Comment thread
djspiewak marked this conversation as resolved.
}
.void
.voidError
.start

action flatMap { fiber =>
supervision.update(_ + ((fiber, result))) map { _ =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't there a possible race condition here? Let's say 2 fibers are starting in parallel:

  • fiber1 is started, observes preempt empty, starts working (but fiber1 is not yet inserted into supervision, that happens concurrently)
  • fiber2 is started, encounters an error, so:
  • fiber2 is completing preempt, and calling cancelAll
  • however, fiber1 is not yet in supervision, so cancelAll will not cancel it. That's not good.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh that's an interesting observation. I think you're right.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

        "interrupt never on error" in ticked { implicit ticker =>
          case object TestException extends RuntimeException
          val test = List(0, 1).parTraverseN(2) {
            case 0 => IO.never[Unit]
            case 1 => IO.raiseError(TestException)
          }

          test.attempt.void.parReplicateA_(100000) must completeAs(())
        }

Weirdly this passes. It feels like we need a double-check on preempt here but the evidence doesn't seem to bear that out.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, it is weird that doesn't hang. However this variant does hang for me (at least I think Succeeded(None) means a hang for ticked):

        "interrupt never on error" in ticked { implicit ticker =>
          case object TestException extends RuntimeException
          val test = (0 to 10).toList.parTraverseN(2) { i =>
            if ((i % 2) != 0) IO.raiseError(TestException)
            else IO.never[Unit]
          }

          test.attempt.void.parReplicateA_(100000) must completeAs(())
        }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm sorry, please disregard my previous comment. As written, that test is expected to sometimes hang.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why would this test sometimes hang? I think I agree with you that it should pass

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So, about my previous test here, with the odd-even never/raiseError: I think it's allowed to hang, because if scheduling happens to work out in a way that 2 instances of never get the 2 available permits, then hanging is expected.

However, since then, I've found this, which also hangs, but really shouldn't:

        "interrupt never on error" in ticked { implicit ticker =>
          case object TestException extends RuntimeException
          val test = (0 to 10).toList.parTraverseN(2) { i =>
            if (i == 5) IO.never[Unit]
            else IO.raiseError(TestException)
          }

          test.attempt.void.parReplicateA_(100000) must completeAs(())
        }

(There is only 1 never here, but 2 permits.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Excellent minimization. It seems very plausible to me that this reproduces the lack of a double-check after adding to the supervision set.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fascinatingly, adding a double-check doesn't fix it. I think this test is actually identifying something else.

result
.get
.flatMap(_.embed(F.canceled *> F.never))
.onCancel(fiber.cancel)
.guarantee(supervision.update(_ - ((fiber, result))))
}
}
}
}
}
}

results.flatMap(_.sequence).onCancel(cancelAll(None))
}
}
}
}

/**
* Like `Parallel.parTraverse_`, but limits the degree of parallelism. Note that the semantics
* of this operation aim to maximise fairness: when a spot to execute becomes available, every
* task has a chance to claim it, and not only the next `n` tasks in `ta`
* Like `Parallel.parTraverse_`, but limits the degree of parallelism. The semantics of this
* function are ordered based on the `Foldable`. The first ''n'' actions will be started
* first, with subsequent actions starting in order as each one completes. Actions which are
* reached earlier in `foldLeftM` order will be started slightly sooner than later actions, in
* a non-blocking fashion. Any errors or self-cancelation will immediately abort the sequence.
* If multiple actions produce errors simultaneously, one of them will be nondeterministically
* selected for production.
*
* The `f` function is run as part of running the action: in parallel and subject to the
* limit.
*/
def parTraverseN_[T[_]: Foldable, A, B](n: Int)(ta: T[A])(f: A => F[B]): F[Unit] = {
require(n >= 1, s"Concurrency limit should be at least 1, was: $n")

Comment thread
djspiewak marked this conversation as resolved.
implicit val F: GenConcurrent[F, E] = this

MiniSemaphore[F](n).flatMap { sem => ta.parTraverse_ { a => sem.withPermit(f(a)) } }
F.deferred[Option[E]] flatMap { preempt =>
F.ref[List[Fiber[F, ?, ?]]](Nil) flatMap { supervision =>
MiniSemaphore[F](n) flatMap { sem =>
val cancelAll = supervision.get.flatMap(_.parTraverse_(_.cancel))

// doesn't complete until every fiber has been at least *started*
val startAll = ta traverse_ { a =>
// first check to see if any of the effects have errored out
// don't bother starting new things if that happens
preempt.tryGet flatMap {
case Some(Some(e)) =>
F.raiseError[Unit](e)

case Some(None) =>
F.canceled

case None =>
F.uncancelable { poll =>
// if the effect produces a non-success, race to kill all the rest
// the laziness is a poor mans defer; this ensures the f gets pushed to the fiber
val wrapped = (F.unit >> f(a)) guaranteeCase {
case Outcome.Succeeded(_) =>
F.unit

case Outcome.Errored(e) =>
preempt.complete(Some(e)).void

case Outcome.Canceled() =>
preempt.complete(None).void
}

// only release the semaphore if we *haven't* errored
val suppressed = wrapped.void.voidError *> sem.release
Comment thread
djspiewak marked this conversation as resolved.

poll(sem.acquire) *> suppressed.start flatMap { fiber =>
// supervision is handled very differently here: we never remove from the set
supervision.update(fiber :: _)
}
}
}
}

// we only run this when we know that supervision is full
val awaitAll = preempt.tryGet flatMap {
case Some(_) => F.unit
case None =>
F.race(
preempt.get.void *> cancelAll,
supervision.get.flatMap(_.traverse_(f => f.join.void).onCancel(cancelAll)))
.void
Comment thread
djspiewak marked this conversation as resolved.
}

// if we hit an error or self-cancelation in any effect, resurface it here
def resurface(poll: Poll[F]) = preempt.tryGet flatMap {
case Some(Some(e)) => F.raiseError[Unit](e)
case Some(None) => poll(F.canceled)
case None => F.unit
}

val work = (startAll *> awaitAll) guaranteeCase {
case Outcome.Succeeded(_) => F.unit
case Outcome.Errored(_) | Outcome.Canceled() => preempt.complete(None) *> cancelAll
}

F.uncancelable(poll => poll(work) *> resurface(poll))
}
}
}
}

override def racePair[A, B](fa: F[A], fb: F[B])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ import scala.collection.immutable.{Queue => ScalaQueue}
* A cut-down version of semaphore used to implement parTraverseN
*/
private[kernel] abstract class MiniSemaphore[F[_]] extends Serializable {
def acquire: F[Unit]
def release: F[Unit]

/**
* Sequence an action while holding a permit
*/
def withPermit[A](fa: F[A]): F[A]
}

Expand Down
31 changes: 31 additions & 0 deletions tests/jvm/src/test/scala/cats/effect/IOPlatformSpecification.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package cats.effect

import cats.effect.std.Semaphore
import cats.effect.syntax.all._
import cats.effect.unsafe.{
IORuntime,
IORuntimeConfig,
Expand Down Expand Up @@ -812,6 +813,36 @@ trait IOPlatformSpecification extends DetectPlatform { self: BaseSpec with Scala
}
}

"parTraverseN" >> {
"short-circuit on error" in real {
case object TestException extends RuntimeException
val target = 0.until(100000).toList
val test = target.parTraverseN(2)(_ => IO.raiseError(TestException))

test.attempt.as(ok).timeoutTo(500.millis, IO(false must beTrue))
}

"interrupt never on error" in ticked { implicit ticker =>
case object TestException extends RuntimeException
val test = List(0, 1).parTraverseN(2) {
case 0 => IO.never[Unit]
case 1 => IO.raiseError(TestException)
}

test.attempt.void.parReplicateA_(100000) must completeAs(())
}
}

"parTraverseN_" >> {
"short-circuit on error" in real {
case object TestException extends RuntimeException
val target = 0.until(100000).toList
val test = target.parTraverseN_(2)(_ => IO.raiseError(TestException))

test.attempt.as(ok).timeoutTo(500.millis, IO(false must beTrue))
}
}

if (javaMajorVersion >= 21)
"block in-place on virtual threads" in real {
val loomExec = classOf[Executors]
Expand Down
Loading
Loading