Skip to content

Commit 4faeac5

Browse files
committed
More efficient round trip conversions for Futures
Makes `toJava.toScala` and `toScala.toJava` a wrap-and-unwrap operation, rather than a wrap-and-rewrap. This won't be possible if we merge scala#46, which speaks in favour of reverting the change that finalized `DefaultPromise` in Scala 2.12.x. This patch does not attempt to "wrap the Future with an implementation of CompletionStage that delegates to Future", as Jame's proposed in issue scala#44, which seems tricky or at least tedious to implement.
1 parent 4b4250a commit 4faeac5

File tree

3 files changed

+38
-10
lines changed

3 files changed

+38
-10
lines changed

src/main/scala/scala/compat/java8/FutureConverters.scala

+16-7
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44
package scala.compat.java8
55

6+
import scala.concurrent.impl.AbstractPromise
67
import scala.concurrent.java8.FuturesConvertersImpl._
78
import scala.concurrent.{ Future, Promise, ExecutionContext, ExecutionContextExecutorService, ExecutionContextExecutor, impl }
89
import java.util.concurrent.{ CompletionStage, Executor, ExecutorService, CompletableFuture }
@@ -54,10 +55,14 @@ object FutureConverters {
5455
* not support the CompletableFuture interface
5556
*/
5657
def toJava[T](f: Future[T]): CompletionStage[T] = {
57-
val cf = new CF[T]
58-
implicit val ec = InternalCallbackExecutor
59-
f onComplete cf
60-
cf
58+
f match {
59+
case p: P[T] => p.wrapped
60+
case _ =>
61+
val cf = new CF[T](f)
62+
implicit val ec = InternalCallbackExecutor
63+
f onComplete cf
64+
cf
65+
}
6166
}
6267

6368
/**
@@ -71,9 +76,13 @@ object FutureConverters {
7176
* @return a Scala Future that represents the CompletionStage's completion
7277
*/
7378
def toScala[T](cs: CompletionStage[T]): Future[T] = {
74-
val p = new P[T]
75-
cs whenComplete p
76-
p.future
79+
cs match {
80+
case cf: CF[T] => cf.wrapped
81+
case _ =>
82+
val p = new P[T](cs)
83+
cs whenComplete p
84+
p.future
85+
}
7786
}
7887

7988
/**

src/main/scala/scala/concurrent/java8/FutureConvertersImpl.scala

+3-3
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ package scala.concurrent.java8
66
// Located in this package to access private[concurrent] members
77

88
import scala.concurrent.{ Future, Promise, ExecutionContext, ExecutionContextExecutorService, ExecutionContextExecutor, impl }
9-
import java.util.concurrent.{ CompletionStage, Executor, ExecutorService, CompletableFuture }
9+
import java.util.concurrent._
1010
import scala.util.{ Try, Success, Failure }
1111
import java.util.function.{ BiConsumer, Function JF, Consumer, BiFunction }
1212

1313
// TODO: make thie private[scala] when genjavadoc allows for that.
1414
object FuturesConvertersImpl {
1515
def InternalCallbackExecutor = Future.InternalCallbackExecutor
1616

17-
class CF[T] extends CompletableFuture[T] with (Try[T] => Unit) {
17+
class CF[T](val wrapped: Future[T]) extends CompletableFuture[T] with (Try[T] => Unit) {
1818
override def apply(t: Try[T]): Unit = t match {
1919
case Success(v) complete(v)
2020
case Failure(e) completeExceptionally(e)
@@ -79,7 +79,7 @@ object FuturesConvertersImpl {
7979
override def toString: String = super[CompletableFuture].toString
8080
}
8181

82-
class P[T] extends impl.Promise.DefaultPromise[T] with BiConsumer[T, Throwable] {
82+
class P[T](val wrapped: CompletionStage[T]) extends impl.Promise.DefaultPromise[T] with BiConsumer[T, Throwable] {
8383
override def accept(v: T, e: Throwable): Unit = {
8484
if (e == null) complete(Success(v))
8585
else complete(Failure(e))

src/test/java/scala/compat/java8/FutureConvertersTest.java

+19
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import static java.util.concurrent.TimeUnit.SECONDS;
1616
import static org.junit.Assert.*;
17+
import static org.junit.Assert.assertSame;
1718
import static scala.compat.java8.FutureConverters.*;
1819

1920
public class FutureConvertersTest {
@@ -341,4 +342,22 @@ public void testToJavaToCompletableFutureDoesNotMutateUnderlyingPromise() throws
341342
assertFalse(sf.isCompleted());
342343
assertFalse(p.isCompleted());
343344
}
345+
346+
@Test
347+
public void testToJavaAndBackAvoidsWrappers() {
348+
final Promise<String> p = promise();
349+
final Future<String> sf = p.future();
350+
final CompletionStage<String> cs = toJava(sf);
351+
Future<String> sf1 = toScala(cs);
352+
assertSame(sf, sf1);
353+
}
354+
355+
@Test
356+
public void testToScalaAndBackAvoidsWrappers() {
357+
final CompletableFuture<String> cf = new CompletableFuture<>();
358+
final Future<String> f = toScala(cf);
359+
CompletionStage<String> cs1 = toJava(f);
360+
assertSame(cf, cs1);
361+
362+
}
344363
}

0 commit comments

Comments
 (0)