Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Use scala.concurrent.Batchable for Scala 2.13 #29986

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright (C) 2009-2021 Lightbend Inc. <https://www.lightbend.com>
*/

package akka.dispatch.internal

import akka.annotation.InternalApi

/**
* INTERNAL API
*/
@InternalApi
private[akka] object ScalaBatchable {

// see Scala 2.13 source tree for explanation
def isBatchable(runnable: Runnable): Boolean = runnable match {
case b: akka.dispatch.Batchable => b.isBatchable
case _: scala.concurrent.OnCompleteRunnable => true
case _ => false
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright (C) 2009-2021 Lightbend Inc. <https://www.lightbend.com>
*/

package akka.dispatch.internal

import akka.annotation.InternalApi

/**
* INTERNAL API
*/
@InternalApi
private[akka] object ScalaBatchable {

/*
* In Scala 2.13.0 `OnCompleteRunnable` was deprecated, and in 2.13.4
* `impl.Promise.Transformation` no longer extend the deprecated `OnCompleteRunnable`
* but instead `scala.concurrent.Batchable` (which anyway extends `OnCompleteRunnable`).
* On top of that we have or own legacy version `akka.dispatch.Batchable`.
*/

def isBatchable(runnable: Runnable): Boolean = runnable match {
case b: akka.dispatch.Batchable => b.isBatchable
case _: scala.concurrent.Batchable => true
Copy link
Member

Choose a reason for hiding this comment

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

I'm sure I fully understand what happened to the Scala internals as described in the comment above. What if this version of Akka is run in an application with Scala 2.13.3 dependency? Will it be caught by this scala.concurrent.Batchable case, or will this be a regression compared to previous OnCompleteRunnable?

We might not care much about that and we can say that they should update to Scala 2.13.4. We already have another reason for that: #30018

Copy link
Member Author

Choose a reason for hiding this comment

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

OnCompleteRunnable extends Batchable in all 2.13.x as far as I can see, so it should not cause a regression in older 2.13 releases.

Copy link
Member

Choose a reason for hiding this comment

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

ok, thanks

case _ => false
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,7 @@ object Envelope {
}

final case class TaskInvocation(eventStream: EventStream, runnable: Runnable, cleanup: () => Unit) extends Batchable {
final override def isBatchable: Boolean = runnable match {
case b: Batchable => b.isBatchable
case _: scala.concurrent.OnCompleteRunnable => true
case _ => false
}
final override def isBatchable: Boolean = akka.dispatch.internal.ScalaBatchable.isBatchable(runnable)

def run(): Unit =
try runnable.run()
Expand Down
15 changes: 9 additions & 6 deletions akka-actor/src/main/scala/akka/dispatch/BatchingExecutor.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,26 @@

package akka.dispatch

import akka.annotation.InternalApi

import java.util.ArrayDeque
import java.util.concurrent.Executor

import scala.annotation.tailrec
import scala.concurrent._

/**
* INTERNAL API
*
* All Batchables are automatically batched when submitted to a BatchingExecutor
*/
@InternalApi
private[akka] trait Batchable extends Runnable {
def isBatchable: Boolean
}

/**
* INTERNAL API
*
* Mixin trait for an Executor
* which groups multiple nested `Runnable.run()` calls
* into a single Runnable passed to the original
Expand Down Expand Up @@ -45,6 +51,7 @@ private[akka] trait Batchable extends Runnable {
* WARNING: The underlying Executor's execute-method must not execute the submitted Runnable
* in the calling thread synchronously. It must enqueue/handoff the Runnable.
*/
@InternalApi
private[akka] trait BatchingExecutor extends Executor {

// invariant: if "_tasksLocal.get ne null" then we are inside Batch.run; if it is null, we are outside
Expand Down Expand Up @@ -127,9 +134,5 @@ private[akka] trait BatchingExecutor extends Executor {
}

/** Override this to define which runnables will be batched. */
def batchable(runnable: Runnable): Boolean = runnable match {
case b: Batchable => b.isBatchable
case _: scala.concurrent.OnCompleteRunnable => true
case _ => false
}
def batchable(runnable: Runnable): Boolean = akka.dispatch.internal.ScalaBatchable.isBatchable(runnable)
}