Add Scala 2.12.0-M2 as a cross build #46
Conversation
|
Review by @rkuhn or @viktorklang |
- `def transform[S](f: Try[T] => Try[S])(implicit executor: ExecutionContext): Future[S]`
- `def transformWith[S](f: Try[T] => Future[S])(implicit executor: ExecutionContext): Future[S]`
- `def flatten[S](implicit ev: T <:< Future[S]): Future[S]`
- `def zipWith[U, R](that: Future[U])(f: (T, U) => R)(implicit executor: ExecutionContext): Future[R]`
Add missing utilities:
- `val unit: Future[Unit]` in `object Future`
- `object never extends Future[Nothing]` in `object Future`
- `def defaultBlockContext: BlockContext` in `object BlockContext`
- `def toString: String` on stdlib implementations of `Future`
Refactors:
- the `scala.concurrent.Future` trait to not explicit create any `Promises`,
so that implementations can control implementation type,
this is mainly facilitated through adding of the `transform` and `transformWith` methods.
- the implementation of `ExecutionContextImpl` has been cleaned up
- the `scala.concurrent.impl.DefaultPromise` has been reimplemented to not use `sun.misc.Unsafe`
Securing:
- Add a self-check in `completeWith` and `tryCompleteWith` to avoid cycles in trait Promise
- Capping the maximum number of threads for the global `ExecutionContext` to the max parallelism
- Implementing (almost) all `Future` combinators on `transformWith` and `transform` means
that `DefaultPromise` linking works on both `(flat)map` and `recover(With)`
- Nested `blocking {}` should not spawn extra threads beyond the first.
Removes:
- the private `internalExecutor` method in favor of an import in trait `Future`
- the private `internalExecutor` method in favor of an import in trait `Promise`
- the `AtomicReferenceFieldUpdater` in `AbstractPromise` since we're using `Unsafe`
- `scala.concurrent.impl.Future` is no longer needed
Deprecates:
- `Future.onSuccess` - discourage the use of callbacks
(and is also redundant considering `foreach` and `onComplete`)
- `Future.onFailure` - discourage the use of callbacks
(and is also redundant considering `onComplete` and `failed.foreach`)
- `ExecutionContext.prepare` - it was ill specced and it is too easy to forget to call it
(or even know when to call it or call it more times than needed)
- All classes in scala.concurrent.forkjoin. Scala 2.12 will be Java 8+ and as such the jsr166e
should be used as included in java.util.concurrent.
Reimplements:
- `failed` - in terms of `transform`
- `map` - in terms of `transform`
- `flatMap` - in terms of `transformWith`
- `recover` - in terms of `transform`
- `recoverWith` - in terms of `transformWith`
- `zip` - in terms of `flatMap` + `map`
- `fallbackTo` - in terms of `recoverWith` + `recoverWith`
- `andThen` - in terms of `transform`
Miscellaneous:
- Giving the threads of `ExecutionContext.global` sensible names
- Optimizes `object Future.successful` and `object Future.failed` are now separate implementations,
to optimize for the result, avoiding doing work for the "other branch".
- Optimizes `compressedRoot()` by avoiding double-calls to volatile get.
Documentation:
- Almost all methods on `Future` and `Promise` have been revisited and had their ScalaDoc updated
Tests:
- Yes
retronym
force-pushed
the
topic/add212
branch
4 times, most recently
from
c374b83 to
770a59a
Compare
And optimize imports.
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.
|
Marking as "on-hold" as this would prevent #50. An alternative would be to unfinalize |
|
@retronym The reason for final-ing DefaultPromise is to make sure that the optimizer can do its job, avoiding tripping into bimorphic or megamorphic callsites. I think the extra allocation and indirection in your proposal is OK given that, but also to prevent others from extending it. Wdyt? |
|
@viktorklang Can you see a way to implement #50 if |
|
We could mark a selection of methods like |
|
@retronym Perhaps an option would be to make all of the methods on DefaultPromise final? |
|
Yeah, I'm fine with that. |
|
@retronym We also need to audit the methods to make sure that nothing will break (is there something that assumes no inheritance?) as well as make sure that methods that may have harmful effects if called from inheriters(?) will be either hidden or appropriately documented. |
|
Yep, changing modifiers is subtle. Could you review your addition of the finality to confirm it was done solely for non functional reasons? |
|
I don't think we need to lock it down to much as it is the whole class already access restricted. If someone plays a trick to subvert that (as we do here) we assume they are responsible for the consequences |
|
I can review the addition at the earliest next week :/ |
|
Released latest genjavadoc (which is |
|
ping @viktorklang for assessment on the impact of unfinalizing |
Avoid inheritance from DefaultPromise, which is final in 2.12.
|
@2m Thanks for that. I've updated this PR to use 0.9 of genjavadoc in both 2.11 and 2.12. |
|
@retronym I only had time for a cursory overview, it seems as unfinalizing it will be fine. May be in order to final-ize all of its methods tho. |
Doing so seems like the cleanest way to enabled conversions like `javaFuture.toScala.toJava` to return the original `javaFuture` in scala-java8-compat. I have made the methods defined in this class final as an alternative lockdown. Discussion, Motivation: scala/scala-java8-compat#46 scala/scala-java8-compat#50
It was non-final in Scala 2.11.x, and made final as part of fa0743c. Removing the final modifier seems like the cleanest way to enable conversions like `javaFuture.toScala.toJava` to return the original `javaFuture` in scala-java8-compat. I have made the methods defined in this class final as an alternative lockdown. Discussion, Motivation: scala/scala-java8-compat#46 scala/scala-java8-compat#50
Add Scala 2.12.0-M2 as a cross build
No description provided.