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.

Sign up for GitHub

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

Jump to bottom

cleanup BatchingExecutor and execution contexts #26690

Open
patriknw opened this issue Apr 5, 2019 · 12 comments
Open

cleanup BatchingExecutor and execution contexts #26690

patriknw opened this issue Apr 5, 2019 · 12 comments
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:core technical-debt

Comments

@patriknw
Copy link
Member

patriknw commented Apr 5, 2019

A few things that I think we can cleanup:

  • Why do we have akka.dispatch.BatchingExecutor? What is the difference between this and then one in Scala?
  • ActorSystem.internalCallingThreadExecutionContext vs akka.dispatch.ExecutionContexts.sameThreadExecutionContext
  • Use 2.13/2.12 specific source files instead of loading with reflection as was introduced as a minimal fix in Use ExecutionContext.parasitic for Scala 2.13, #26655 #26683, at least 2.13 parasitic is public
@patriknw patriknw added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:core technical-debt labels Apr 5, 2019
@patriknw patriknw added this to the 2.6 milestone Apr 5, 2019
@viktorklang
Copy link
Member

  1. The one in Scala is not public API
  2. I'd remove ActorSystem.internalCallingThreadExecutionContext and have akka.dispatch.ExecutionContexts.sameThreadExecutionContext delegate to ExecutionContext.parasitic (or try to load the Future.InternalCallbackExecutor if not on 2.13.
  3. Using specific source files per Scala version sounds like a good plan to avoid reflection.

@patriknw
Copy link
Member Author

patriknw commented Apr 5, 2019

Thanks for input

@viktorklang
Copy link
Member

Might also want to add a comment to Akka's BatchingExecutor to NOT do any bugfixes directly there, but instead open up an Issue against scala. And just treat the code in Akka as a "shaded" class

@jrudolph jrudolph mentioned this issue Jul 8, 2019
Open
@johanandren johanandren self-assigned this Jan 23, 2020
@johanandren johanandren added 3 - in progress Someone is working on this ticket and removed 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted labels Jan 23, 2020
johanandren added a commit to johanandren/akka that referenced this issue Jan 23, 2020
@johanandren
Use the same thread ec from the respective Scala version standard lib…
82888cd 
…raries akka#26690
@johanandren johanandren mentioned this issue Jan 23, 2020
Merged
johanandren added a commit to johanandren/akka that referenced this issue Jan 23, 2020
@johanandren
Use the same thread ec from the respective Scala version standard lib…
641e2a7 
…raries akka#26690
johanandren added a commit to johanandren/akka that referenced this issue Jan 23, 2020
@johanandren
Use the same thread ec from the respective Scala version standard lib…
f2e9110 
…raries akka#26690
johanandren added a commit that referenced this issue Mar 10, 2020
@johanandren
Clean up same thread execution contexts #26690
d26453b 
* deprecate internal sameThread ec and use a new one for all internal use sites
* Use the respective Scala version standard library "same thread" ec 
* fallback to the old inline impl on 2.12 when reflection isn't possible
@johanandren
Copy link
Member

johanandren commented Mar 10, 2020 •
edited

Using the execution context from the respective Scala version implemented. Doing something with the BatchingExecutor remains (if we can at all do anything about it)

@patriknw patriknw modified the milestones: 2.6, 2.6.4 Mar 12, 2020
@patriknw
Copy link
Member Author

@johanandren was this fully completed or something remaining?

@johanandren
Copy link
Member

No, only partially completed, see my comment above

@johanandren johanandren reopened this Mar 12, 2020
@johanandren johanandren removed this from the 2.6.4 milestone Mar 12, 2020
@patriknw
Copy link
Member Author

ok, thanks

@johanandren
Copy link
Member

Perhaps we should be careful with backporting the BatchingExecutor given scala/bug#12089

@johanandren
Copy link
Member

Looking into this a little bit more the current batching impl we have has the same behavior as the 2.13.2 one, so it probably doesn't matter if we update to a faster one with the same behavior.

@mushtaq
Copy link

mushtaq commented Jul 22, 2020

Looking into this a little bit more the current batching impl we have has the same behavior as the 2.13.2 one, so it probably doesn't matter if we update to a faster one with the same behavior.

@johanandren, @patriknw Can you please confirm if ActorSystem[_].executionContext in Akka 2.6.8 (with Scala 2.13.2) will have the same issue as of Scala's global ExecutionContext raised in scala/bug#12089?

@johanandren
Copy link
Member

@mushtaq Yes it will, and has behaved like this for a very long while. See #29415 that I just created as well.

@viktorklang
Copy link
Member

viktorklang commented Jul 22, 2020 via email

@johanandren johanandren mentioned this issue Jan 27, 2021
Closed
@johanandren johanandren added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted and removed 3 - in progress Someone is working on this ticket labels Mar 8, 2022
@johanandren johanandren removed their assignment Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:core technical-debt
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mushtaq @viktorklang @patriknw @johanandren