Add synchronous ("parasitic") ExecutionContext #7784

viktorklang · 2019-02-22T16:49:03Z

A synchronous, trampolining, ExecutionContext has been used for a long time within the Future implementation to run controlled logic as cheaply as possible.

I believe that there is a significant number of use-cases where it makes sense, for efficiency, to execute logic synchronously in a safe(-ish) way without having users to implement the logic for that ExecutionContext themselves—it is tricky to implement to say the least.

It is important to remember that ExecutionContext should be supplied via an implicit parameter, so that the caller can decide where logic should be executed. The use of ExecutionContext.parasitic means that logic may end up running on Threads/Pools that were not designed or intended to run specified logic. For instance, you may end up running CPU-bound logic on an IO-designed pool or vice versa. So use of parasitic is only advisable when it really makes sense. There is also a real risk of hitting StackOverflowErrors for certain patterns of nested invocations where a deep call chain ends up in the parasitic executor, leading to even more stack usage in the subsequent execution. Currently the parasitic ExecutionContext will allow a nested sequence of invocations at max 16, this may be changed in the future if it is discovered to cause problems.

huntc · 2019-02-22T19:36:50Z

Seems like a good idea. Context switching can be hell, particularly when there are few resources available and it makes no sense to context switch for just a map etc ie a single core. I think I’d find myself using this in most places.

lihaoyi · 2019-02-23T17:32:47Z

CC @sjrd I vaguely remember we discussed this half a decade ago around the time scala-js/scala-js#2102 happened

lihaoyi · 2019-02-23T17:33:41Z

Also relevant https://grokbase.com/t/gg/scala-user/1421nzzvvb/should-there-be-a-runnow-executioncontext-in-executioncontext-implicits

NthPortal · 2019-02-24T04:26:08Z

src/library/scala/concurrent/ExecutionContext.scala

+   * Nested submissions will be trampolined to prevent uncontrolled stack space growth.
+   * Any `NonFatal` or `InterruptedException`s will be reported to the `defaultReporter`.
+   *
+   * It is advised not to call any blocking code in the `Runnable`s submitted to this `ExecutionContext`


this warning honestly doesn't seem strong enough to me. is there any valid situation where you should call blocking code using this ExecutionContext?

I personally lean towards something like "DO NOT call any blocking code ..."

Great point, @NthPortal. I have updated the Scaladoc—what do you think of the new text?

WellingR · 2019-02-24T13:56:10Z

src/library/scala/concurrent/BatchingExecutor.scala

        try submitForExecution(runnable) // User code so needs to be try-finally guarded here
+        catch {
+          case ie: InterruptedException =>
+            reportFailure(ie) // TODO: Handle InterruptedException differently?


If you catch an InterruptedException shouldn't you interrupt the current thread again?

@WellingR I've been back-and-forth on that one, since control is handed back to the "pool" already. Question is when it should be reset—since we cannot not execute any of the Runnables since then they will never be executed.

@WellingR I just also remembered: If there's an InterruptedException thrown by user logic, they're using this EC to run blocking logic—which is documented to not be done. :/

mdedetrich · 2019-02-24T23:35:11Z

I think it makes sense for making this public, I mean the current situation with Future has gotten so ridiculous that akka ended up having to implement a "fast" Future to solve this exact problem (i.e. having to use an ExecutionContext for small .map operations).

I am also wondering whether having control over trampolining or not should be desired for those cases where you need to squeeze even more performance.

jroper · 2019-02-25T01:04:22Z

src/library/scala/concurrent/ExecutionContext.scala

+   * advised to only execute logic which will quickly return control to the caller.
+   *
+   * DO NOT call any blocking code in the `Runnable`s submitted to this `ExecutionContext`
+   * as it will prevent progress by other enqueued `Runnable`s and the calling `Thread`.


I like that the potential performance improvements from this over a thread pool executor are not mentioned - the last thing we want is people seeing this and saying "oh magic performance improvement secret sauce! I'm going to use this!" without reading the rest of the docs. Also, I wonder if we can be even clearer about the need for care. Something like "Symptoms of misusing this ExecutionContext include deadlocks and a potential for your application to slow to a crawl under minimal load."

@jroper Great points, James. I'll craft a message about symptoms of misuse.

@jroper The new working name is parasitic—please check out the new docs. :)

retronym · 2019-02-25T03:23:11Z

src/library/scala/concurrent/ExecutionContext.scala

+  final object callingThread extends ExecutionContextExecutor with BatchingExecutor {
+    override final def submitForExecution(runnable: Runnable): Unit = runnable.run()
+    override final def execute(runnable: Runnable): Unit = submitSyncBatched(runnable)
+    override final def reportFailure(t: Throwable): Unit =


Should we add explicit memory fences so that this execution context establishes the happens-before relationships that the normal one would?

I don't think it's necessary.

There's no need for a memory barrier since it's guaranteed that the runnable will run on the calling thread, so there is an intrinsic happens before relationship there. So, as long as any state captured by the runnable is appropriately synchronised before being submitted to the executor (which this executor can't do anything about, memory barrier or not, if it's not), then everything should be fine.

So if we then think about this in the context of the Scala Future API, the DefaultPromise implementation uses AtomicReference.compareAndSet to query for completion state and/or add callbacks to be executed (eg in map) and remove them before executing them again (eg in complete). There is a memory barrier here (eg, on x86 its implemented using CMPXCHG which gives a memory barrier), so any state held by callbacks that then eventually get submitted to this executor will have their state synchronised so as to guarantee happens before before being submitted to the executor. Other implementations of Future likewise must implement a memory barrier on all their callback capturing methods, if they don't then adding a memory in this executor will only save them in a subset of situations, specifically, it couldn't save them, for example, if callbacks captured by map didn't have a memory barrier, since by the time this executor gets involved, the calling thread that registered that callback is gone and its memory cannot be synchronised.

@retronym @jroper Yes, since the execution does not leave the calling thread then there is no need for any fences.

viktorklang · 2019-02-25T08:49:47Z

@lihaoyi Thanks for reminding me of that 5 year old conversation! :-) I guess I've grown more flexible in my thinking over the years :)

viktorklang · 2019-02-25T09:19:51Z

src/library/scala/concurrent/ExecutionContext.scala

+   * as it will prevent progress by other enqueued `Runnable`s and the calling `Thread`.
+   *
+   * Symptoms of misuse of this `ExecutionContext` include, but are not limited to, deadlocks
+   * and severe performance problems.


@jroper @NthPortal Is this an adequate explanation?

viktorklang · 2019-02-25T13:02:30Z

It would seem like there are basically only positive feedback on this proposal, so I have switched it over to Ready for review.

sjrd · 2019-02-25T13:18:53Z

I am a bit worried that this will set wrong expectations, not in terms of performance, but in terms of correctness. Asynchronous code, requesting implicit ExecutionContexts, does not always follow the EC for all operations. Sometimes, they delegate to other asynchronous APIs, and provide callbacks in which they will complete Promises. Such code will still complete asynchronously, even if you call it with a callingThread EC. If a user calls such code with a callingThread EC and then expects the returned Future to be synchronously completed, their code won't work. It's even more annoying that the implementation could subtly change in later versions of a library, and then code that used to be OK by chance will break.

This is something we've experienced in Scala.js with the runNow execution context we used to have, and is one of the reasons we deprecated it, and removed it in 1.x.

viktorklang · 2019-02-25T13:31:25Z

@sjrd I think that's a good point. I think the name runNow gives a bit different expectations than callingThread—but I agree that it should be better spelled out in the documentation for it. Do you have any suggestion as what would be the clearest formulation of such text?

sjrd · 2019-02-25T15:51:18Z

Something like

Using callingThread does not give any guarantee that the resulting Future[T] will be completed by the time the callee returns. The caller must still use combinators and/or onComplete-like methods to process the result of the Future when it becomes ready.

The wording can probably be improved, but the above should convey the message that I think is important.

viktorklang · 2019-02-25T18:36:20Z

@sjrd Ok, thank you! Since ECs are not exclusive to Futures, I was struggling a bit to find the optimal place to add such documentation—but I guess it's fine to add it in the callingThread EC as it is most likely that it will be used for such code.

NthPortal · 2019-02-28T00:12:38Z

It would seem like there are basically only positive feedback on this proposal

I have mixed feelings, but it took me a while to figure out what I wanted to say.

First, there is the concern that where callingThread executes is non-deterministic. If you perform some operation on a Future, and it is already completed when the operation is performed, that operation will be executed on the current thread; otherwise, the operation will be executed by whatever ExecutionContext completes the Future. This non-determinism can lead to nasty bugs (as noted by Sébastien and others).

Second, it allows one to perform operations with an ExecutionContext they would not otherwise have access to. If the operation performed using callingThread is too expensive, it may slow or starve the ExecutionContext. While it would be possible to write your own implementation of callingThread and access the same inaccessible ExecutionContext, making it part of the standard library makes it significantly more accessible and more likely to be misused. If people have to implement it themselves, it will be mostly only experts who know when it's safe to use it implementing it; if it's in the standard library, anyone might think it's a good idea to use (when it isn't). This change makes it very easy to starve someone else's ExecutionContext.

I am personally more concerned about the second problem, though both are important. I think both problems can be mostly addressed by good documentation, though I also wonder if it is possible or a good idea to make it somehow less accessible (a worse name? nested inside something?). I will try to review the docs more this weekend.

viktorklang · 2019-02-28T08:00:34Z

@NthPortal Yes, I agree with everything you wrote. I think improving the documentation is the right thing to do. It is worth noting that none of the issues you mention is new capabilities (users can already write this logic themselves to starve or disrupt other ExecutionContexts). Perhaps with better documentation, and possibly another (more discouraging?) name, we can arrive at a solution which is to be deemed acceptable?

tarsa · 2019-02-28T22:17:34Z

Perhaps with better documentation, and possibly another (more discouraging?) name, we can arrive at a solution which is to be deemed acceptable?

What about ExecutionContext.threadless or even ExecutionContext.parasitic? With documentation like:

This ExecutionContext latches on all threads it is called from. Threads are blocked until their thread local task queues are empty. When this ExecutionContext is used indirectly (through some higher level abstraction like Future) it can be unpredictable to which threads it will latch on. Possibly it can block threads that are critical to application stability and responsiveness.

That description actually explains the mechanics and the problem with careless usage.

viktorklang · 2019-03-02T19:18:25Z

I’d be ok with parasitic too. Has a name which leads the user to read its docs. -- Cheers, √

viktorklang · 2019-03-02T20:54:13Z

I've committed an update to parasitic, have a look at it and the revised docs: cc5ae2a

NthPortal · 2019-03-02T21:30:30Z

src/library/scala/concurrent/ExecutionContext.scala

   * @return the global `ExecutionContext`
   */
-  def global: ExecutionContextExecutor = Implicits.global.asInstanceOf[ExecutionContextExecutor]
+  final def global: ExecutionContextExecutor = Implicits.global.asInstanceOf[ExecutionContextExecutor]


I'm having trouble understanding why this reads from Implicits.global and does a cast, rather than Implicits.global reading from this and not needing to cast (not from this PR, ik)

@NthPortal Great question! It's because Implicits.global needs to be typed ExecutionContext and not ExecutionContextExecutor otherwise its more specific type would take precedence over other implicit ExecutionContexts in scope. The non-implicit ExecutionContext.global can be typed as ExecutionContextExecutor so it can be used for Java APIs requiring Executor.

@viktorklang what I meant was, why not instead do this?

lazy final val global: ExecutionContextExecutor = impl.ExecutionContextImpl.fromExecutor(null: Executor) object Implicits { implicit final def global: ExecutionContext = ExecutionContext.global // no cast needed! }

@NthPortal Done!

NthPortal · 2019-03-02T22:35:25Z

src/library/scala/concurrent/ExecutionContext.scala

+   * WARNING: Do *not* call any blocking code in the `Runnable`s submitted to this `ExecutionContext`
+   * as it will prevent progress by other enqueued `Runnable`s and the calling `Thread`.
+   * In order to maximize application responsiveness, it is strongly advised
+   * to only execute logic which will quickly return control to the caller.


Is it necessary to say 'In order to maximize application responsiveness' and that it's 'strongly advised'? What are your thoughts on shortening the sentence to just 'Only execute logic which will quickly return control to the caller'?

@NthPortal Updated! :)

NthPortal

I like this approach. I think most people, upon seeing parasitic, will say "I don't think I want to use that one," which is what we want. There might be some shed painting on the exact phrasing of the docs, but overall looks great to me.

NthPortal · 2019-03-03T00:02:05Z

@viktorklang looks like messing with which is the primary definition of global changed the phrasing of a partest output

  t8849.scala:8: error: ambiguous implicit values:
- both lazy value global in object Implicits of type => scala.concurrent.ExecutionContext
+ both method global in object Implicits of type => scala.concurrent.ExecutionContext

viktorklang · 2019-03-03T15:17:13Z

@NthPortal Fixed t8849

tarsa · 2019-03-03T17:10:06Z

Unless I missed something, documentation still doesn't seem to mention unpredictable outcomes when using this ExecutionContext through abstractions, i.e. I mean this bit:

When this ExecutionContext is used indirectly (through some higher level abstraction like Future) it can be unpredictable to which threads it will latch on. Possibly it can block threads that are critical to application stability and responsiveness.

Wording can be changed to be consistent with the rest of documentation, e.g. "latch on" -> "steal time".

viktorklang · 2019-03-03T18:53:04Z

Piotr, I have updated the docs to include a mention about Futures. Have a look! -- Cheers, √

tarsa · 2019-03-03T19:09:56Z

Yep, looks good.
👍

src/library/scala/concurrent/Future.scala

lihaoyi · 2019-03-06T08:50:30Z

Here's a bit of a wild idea: would it be possible to make an ExecutionContext that is smart enough to automatically trampoline operations on the same thread, IFF the operations are given the same execution context to run on (by reference equality), and where the ExecutionContext's differ then going through the normal flow?

That would allow high performance in the common case where a bunch of maps and flatMaps share an ExecutionContext, while still preserving good behavior in the case where we're handing over our asynchronous workflows between different ExecutionContexts

tarsa · 2019-03-06T09:05:12Z

@lihaoyi
Documentation says:

Nested invocations of execute will be trampolined to prevent uncontrolled stack space growth.

Isn't that what you want?

Other executors also have batching. I'm not sure how it works in detail, but IIUC a single batch is executed on single thread.

viktorklang · 2019-03-06T09:43:35Z

@lihaoyi Yes, that is implemented in #7470 :-)

This is a specific ExecutionContext which executes submitted Runnables on the calling thread, utilizing a combination of limited stack growth with on-heap trampolining to achieve its functionality and prevents unbounded stack growth. It is introduced as it is too easy for end-users to implement equivalent functionality but it is tricky to get right.

…icits.global

viktorklang · 2019-03-07T17:54:48Z

Squashed some of the commits here. Will merge after CI OKs it.

SethTisue · 2019-03-07T20:35:45Z

@viktorklang people will find their way here from the release notes, so you may want to revise the PR description to reflect the final state

viktorklang · 2019-03-07T20:44:50Z

@SethTisue Thanks Seth—I've revised the PR description.

Changes from: * scala/scala#7784 * scala/scala#7062

Changes from: * scala/scala#7784 * scala/scala#7062 * scala/scala#7670

SethTisue · 2024-06-21T04:43:38Z

new blog post by @wsargent about ExecutionContext.global, ExecutionContext.parasitic, and ExecutionContext.opportunistic:

https://tersesystems.com/blog/2024/06/20/executioncontext.parasitic-and-friends/

viktorklang added this to the 2.13.0-RC1 milestone Feb 22, 2019

viktorklang self-assigned this Feb 22, 2019

This comment has been minimized.

Sign in to view

viktorklang requested review from NthPortal and retronym February 23, 2019 16:18

NthPortal reviewed Feb 24, 2019

View reviewed changes

WellingR reviewed Feb 24, 2019

View reviewed changes

jroper reviewed Feb 25, 2019

View reviewed changes

retronym reviewed Feb 25, 2019

View reviewed changes

viktorklang force-pushed the wip-callingthread-ec-√ branch from 66d4b6e to e2aee62 Compare February 25, 2019 09:19

viktorklang commented Feb 25, 2019

View reviewed changes

viktorklang marked this pull request as ready for review February 25, 2019 13:02

viktorklang added performance the need for speed. usually compiler performance, sometimes runtime performance. and removed performance the need for speed. usually compiler performance, sometimes runtime performance. labels Feb 25, 2019

viktorklang force-pushed the wip-callingthread-ec-√ branch from f71eb56 to 0205ca9 Compare February 27, 2019 22:32

NthPortal reviewed Mar 2, 2019

View reviewed changes

NthPortal approved these changes Mar 2, 2019

View reviewed changes

viktorklang force-pushed the wip-callingthread-ec-√ branch from cc5ae2a to 310e54d Compare March 2, 2019 23:02

WellingR approved these changes Mar 3, 2019

View reviewed changes

NthPortal reviewed Mar 3, 2019

View reviewed changes

src/library/scala/concurrent/Future.scala Show resolved Hide resolved

viktorklang added 2 commits March 7, 2019 18:52

Switches the implementation order of ExecutionContext.global and Impl…

bc11322

…icits.global

viktorklang force-pushed the wip-callingthread-ec-√ branch from 75d2433 to bc11322 Compare March 7, 2019 17:53

SethTisue changed the title ~~FOR DISCUSSION: Synchronous (callingThread) ExecutionContext~~ Add synchronous ("parasitic") ExecutionContext Mar 7, 2019

SethTisue added the release-notes worth highlighting in next release notes label Mar 7, 2019

SethTisue merged commit 1ab9e39 into scala:2.13.x Mar 7, 2019

sjrd added a commit to sjrd/scala-js that referenced this pull request Mar 11, 2019

Update overrides for the latest 2.13 nightly.

e668732

Changes from: * scala/scala#7784 * scala/scala#7062

sjrd mentioned this pull request Mar 11, 2019

Update overrides for the latest 2.13 nightly. scala-js/scala-js#3581

Merged

sjrd added a commit to sjrd/scala-js that referenced this pull request Mar 11, 2019

Update overrides for the latest 2.13 nightly.

8e13590

Changes from: * scala/scala#7784 * scala/scala#7062 * scala/scala#7670

diesalbla added the library:concurrent Changes to the concurrency support in stdlib label Mar 15, 2019

Add synchronous ("parasitic") ExecutionContext #7784

Add synchronous ("parasitic") ExecutionContext #7784

Uh oh!

Conversation

viktorklang commented Feb 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

huntc commented Feb 22, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

lihaoyi commented Feb 23, 2019

Uh oh!

lihaoyi commented Feb 23, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdedetrich commented Feb 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jroper Feb 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viktorklang commented Feb 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viktorklang commented Feb 25, 2019

Uh oh!

sjrd commented Feb 25, 2019

Uh oh!

viktorklang commented Feb 25, 2019

Uh oh!

sjrd commented Feb 25, 2019

Uh oh!

viktorklang commented Feb 25, 2019

Uh oh!

NthPortal commented Feb 28, 2019

Uh oh!

viktorklang commented Feb 28, 2019

Uh oh!

tarsa commented Feb 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

viktorklang commented Mar 2, 2019 via email

Uh oh!

viktorklang commented Mar 2, 2019

Uh oh!

NthPortal Mar 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

viktorklang commented Feb 22, 2019 •

edited

Loading

jroper Feb 25, 2019 •

edited

Loading

tarsa commented Feb 28, 2019 •

edited

Loading

NthPortal Mar 2, 2019 •

edited

Loading

lihaoyi commented Mar 6, 2019 •

edited

Loading

tarsa commented Mar 6, 2019 •

edited

Loading