refactor: New API for work-stealing from QueueExecutionContext
#3853
Conversation
nativelib/src/main/scala/scala/scalanative/runtime/NativeExecutionContext.scala
Outdated
Show resolved
Hide resolved
|
@WojciechMazur this still doesn't allow to implement integration with event loop. An example of implementation before your refactoring: private def loopRunOnce(): Boolean = {
if (queue.nonEmpty) {
val runnable = queue.remove(0)
try runnable.run()
catch { case t: Throwable => ExecutionContext.global.reportFailure(t) }
if (uv_loop_alive(uvLoop) != 0) {
uv_update_time(uvLoop)
uv_run(uvLoop, uv_run_mode.NOWAIT)
}
} else if (uv_loop_alive(uvLoop) != 0) uv_run(uvLoop, uv_run_mode.ONCE)
(uv_loop_alive(uvLoop) != 0) || queue.nonEmpty
}here I should know has the executor got anything inside to run it, or I may block until the event on the loop. With this PR it still impossible to make. |
| override def stealWork(maxSteals: Int): Unit = { | ||
| if (maxSteals <= 0) () | ||
| else { | ||
| var steals = 0 | ||
| while (steals < maxSteals && hasAvailableTasks) { | ||
| doStealWork() | ||
| steals += 1 | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
| override def stealWork(maxSteals: Int): Unit = { | |
| if (maxSteals <= 0) () | |
| else { | |
| var steals = 0 | |
| while (steals < maxSteals && hasAvailableTasks) { | |
| doStealWork() | |
| steals += 1 | |
| } | |
| } | |
| } | |
| override def stealWork(maxSteals: Int): Boolean = { | |
| if (maxSteals > 0) { | |
| var steals = 0 | |
| while (steals < maxSteals && hasAvailableTasks) { | |
| doStealWork() | |
| steals += 1 | |
| } | |
| } | |
| queue.hasAvailableTasks | |
| } |
like this allows to implement libuv integration without any issue
There was a problem hiding this comment.
Maybe it's the issue with taking look at it at 2 AM, but I don't really get how returning boolean helps here. Wouldn't having an additional call to public method returning the same value would be feasible? Especially in context of previous snippet with libuv usage seems to confirm that, becouse I assume it could be rewritten as following I suppose:
val queue = QueueExecutionContext
private def loopUV(): Boolean = {
queue.stealWork(1) // execute first or noop
if (queue.hasAvailableTasks) {
if (uv_loop_alive(uvLoop) != 0) {
uv_update_time(uvLoop)
uv_run(uvLoop, uv_run_mode.NOWAIT)
}
} else if (uv_loop_alive(uvLoop) != 0) uv_run(uvLoop, uv_run_mode.ONCE)
(uv_loop_alive(uvLoop) != 0) || queue.hasAvailableTasks
}Disclaimer: I do miss context about usage of libuv, I have no experience in it and don't really know how you're integrating it in rest of the code
My main issue with making the stealWork return Boolean is the fact that it might be confusing. I'd rather expect boolean to symbolize that it have stealed some work instead of informing that it can steal more tasks
There was a problem hiding this comment.
Yes, making hasAvailableTasks public helps here.
Regarding original issue or some context: libuv is just an example event-loop. You may think about it as a pool or select. The goal is mix poll/select with scala-native event loop with trivial logic:
- if SN has something else, run multiplexor without blocking to fetch that it has already;
- if SN hasn't got anything else, block on it until something happened.
…implicit def queue: ExecutionContext`
|
Thanks for working on this API!
I think we should think about this now before this API is finalized. For example, consider that the runtime supports the enqueuing of timers. How should the timer stealing API look like? If there is a timer scheduled, but it will not fire until the future, does For any user application built on top of this API to be backwards-compatible when scheduling capabilities are introduced, we must put in the hooks now. Similarly, what about I/O? It should be possible to support |
|
Ok, have to admit that with the remaining part it might not be so trivial.
If we don't want to force any particular dependencies, then for timers we can simply add dedicate queue like you do in CE, and then execute or move timed out timers to the front of compute-queue. We can inject at the begining of the
Tough question. If we assume to EC is just a computation bound then it shall return false. Maybe an additional
At that point we'd need to also provide the whole support based on epoll, kqueue or overlapped operations on Windows. Doable, but would take time. And we're already behind the schedule. As long as we're dealing with only computation-bound queue it's fine, but when we dig into the I/O, async stuff then we quickly get to the point, where we need to have a fully functional event loop. I'll try to dig into these areas tomorrow, altough I hoped we would be able to make a release soon. |
I agree, and as I have said before I do not think that there is an urgency:
Until Scheduling and I/O APIs land in Scala stdlib it might be difficult to know exactly how it should look like.
Yes, indeed. That is the point of the interfaces that we are proposing to add to stdlib: to create an event loop abstraction. The power of this abstraction is that would be cross-platform (JVM / JS / {single,multi}-threaded Native) and async framework agnostic (Future / IO / ZIO / Gears / Ox), the same way that
Along these lines, I think it is important to take a step back and consider why we are even adding these APIs anyway. The current proposed usecase for these APIs is to make it possible to interleave two event loops ( In Scala, hard-coding an |
As a guy who had implemented Scheduling and async I/O for Scala-Native via libuv and who struggle with porting this to 0.5.0: I plan to release under AGPL the mail repository which I'm working quite soon (I really hope, really-really) but this part isn't critical for solution and I can extract it under public domain which allows to use it everywhere, like I did for SHA and Blake3.
|
Is there any work actually being done on this, or is this just an hypothetical? The last comment on the contributors thread is more than one year old. Maybe it's OK to just go ahead with this change and then, if needed, implement the Scheduling API on 0.6.x? I think it's the 3rd time this feature is implemented, so there's clearly some interest in this, and I fear that we are letting perfect be the enemy of good. |
There was a problem hiding this comment.
@WojciechMazur thanks for new API. It makes me very happy that ugly hack with rawPtr finaly removed.
Yes. The necessary changes are being made to tooling so that the stdlib is open for additions. As soon as it is, I intend to propose these new interfaces. See Discord thread.
Right, but interest does not justify adding it. As I wrote in #3853 (comment), are we sure that these APIs are necessary or just a workaround for bad practices? Basically I would like to understand if there is a better explanation than "libraries should be able to hard-code @catap thoughts? |
Well, if we move into implementing NIO, we defently need something as dummy inside Scala.JS to make ecosystem complete. I do have some wrappers around NIO which works with SJS and SN, but it quite limmited. Frankly speaking this task quite amazing and huge, special in case of debuging it and make it stable enoug.
Without this "stealing work" API, it is impossible to mix SN event loop and event loop for socket via libuv or similar library. When multithreading works and availble, an event loop can be moved to dedicated thread and no need of such API. But not all platform is supporting multithreading now, and some new platform may not support it. So, we need a way to allow it for such platform at least. |
I agree. However, my question is why is it necessary to mix the event loops to begin with? |
|
On Sun, 31 Mar 2024 17:56:47 +0200, Arman Bilge wrote:
I agree. However, my question is _why_ is it necessary to mix the event loops to begin with?
Without such mixing it is impossible to create any server which.
Have I missed something?
…--
wbr, Kirill
|
@catap Have you looked at the Typelevel stack? We've had a working HTTP stack since 2022 without using any "hacks" or mixing event loops. You may also be interested to study the implementation because it supports NIO without libuv. |
|
On Sun, 31 Mar 2024 18:32:39 +0200, Arman Bilge wrote:
> Without such mixing it is impossible to create any server which.
@catap Have you looked at the Typelevel stack? [We've had a working HTTP stack since 2022](https://typelevel.org/blog/2022/09/19/typelevel-native.html) without using any "hacks". You may also be interested to study the implementation because it implements NIO layer without libuv.
At first I think to say no, but started to look and I recall this.
Instead you have implemented your own ExecutorContext: https://github.com/armanbilge/epollcat/tree/main/core/src/main/scala/epollcat/unsafe
And the hack is simple moved into it.
In my case it works with standard execution context, like scala-native-loop
does, which seems cleaner and more user friendly.
Or at least I think so.
…--
wbr, Kirill
|
Right, so if I understand correctly you are confirming what I already said #3853 (comment):
As I said, I believe this API is being introduced as a workaround for poor library design that does not follow best practices. I do not think that is a good justification to introduce this API. |
@catap just to clarify: by "standard" do you mean "global", or do you mean |
What hack? There is no hack. |
|
On Sun, 31 Mar 2024 18:54:41 +0200, Arman Bilge wrote:
> Basically I would like to understand if there is a better explanation than "libraries should be able to hard-code `ExecutionContext.global` even though it is a bad practice and other libraries should be able to work around that via these APIs". Because in my opinion this is a poor reason to add these APIs.
As I said, I believe this API is being introduced as a workaround for poor library design that does not follow best practices. I do not think that is a good justification to introduce this API.
But I think that it poor design of library which enforce to use specifed
ExecutionContext which is provided as part of this library.
Let assume that we have two libraries L1 and L2, and both requires to use
their own ExecutionContext. We are doom, aren't we?
Where I stand to enforce using standr aka global ExecutionContext special
for the case of not multithreading application is much better approach.
…--
wbr, Kirill
|
As "standrd" I mean |
Yes. This is because the libraries have not followed best practices. Best practice is to never hardcode an If L1 and L2 both accept an |
|
On Sun, 31 Mar 2024 19:07:21 +0200, Arman Bilge wrote:
> Let assume that we have two libraries L1 and L2, and both requires to use their own ExecutionContext. We are doom, aren't we?
Yes. This is because the libraries have not followed best practices. Best practice is to _never_ hardcode an `ExecutionContext`, including `ExecutionContext.global`. Best practice is to instead take an `implicit ec: ExecutionContext` as a parameter. See for example the design of `Future`.
If L1 and L2 both accept an `ExecutionContext` as a parameter, then you can share the same `ExecutionContext` between them.
L1 requires to use L1ExecutionContext, L2 requires L2ExecutionContext. Which
should user use to be able to use both of them?
…--
wbr, Kirill
|
I'm not sure what you mean by "require". Can you give an example?
trait ExecutionContext {
def execute(runnable: Runnable): Unit
def reportFailure(cause: Throwable): Unit
}Suppose that trait L1ExecutionContext extends ExecutionContext {
def doL1Thing(runnable: Runnable): Unit
}
trait L2ExecutionContext extends ExecutionContext {
def doL2Thing(runnable: Runnable): Unit
}In this case, I completely agree with you that it would be impossible for L1 or L2 to use the other library's But if that is the case you are in, I do not see how the changes in this PR will help you at all. The fundamental problem is that you cannot mix two event loops in a single-threaded application. There is no way around that. |
|
On Sun, 31 Mar 2024 19:28:19 +0200, Arman Bilge wrote:
I'm not sure what you mean by "require". Can you give an example?
`ExecutionContext` is an abstraction:
```scala
trait ExecutionContext {
def execute(runnable: Runnable): Unit
def reportFailure(cause: Throwable): Unit
}
```
Suppose that `L1ExecutionContext` and `L2ExecutionContext` both add extra methods to the interface, for example:
```scala
trait L1ExecutionContext extends ExecutionContext {
def doL1Thing(runnable: Runnable): Unit
}
trait L2ExecutionContext extends ExecutionContext {
def doL2Thing(runnable: Runnable): Unit
}
```
In this case, I completely agree with you that it would be impossible for L1 or L2 to use the other library's `ExecutionContext`. You are indeed "doomed".
But if that is the case you are in, I do not see how the changes in this PR will help you at all. The fundamental problem is that you cannot mix two event loops in a single-threaded application. There is no way around that.
Good example, and scala.scalanative.runtime.NativeExecutionContext is just
another implementation of ExecutionContext which adds some methods to be
easy integrated into someone's event loop or similar thing.
Frankly, I don't understand the reason of this discussion.
…--
wbr, Kirill
|
Yes, and this is not necessary. If some library L1 is able to use the "standard" global
Me neither :) I'm afraid we are talking past each other. I understand your goals are to implement a single-threaded server. This is possible without the changes in this PR and without the hack you are currently using. I know this because this is exactly what we've done in Typelevel. |
|
I was thinking a bit more about this last night, and I still have some questions regarding rejecting this: First, SIP-51 only proposes dropping the forwards binary compatibility for the 2.13 stdlib, so even if the Second, regarding, "libraries should be able to hard-code ExecutionContext.global even though it is a bad practice and other libraries should be able to work around that via these APIs", I think this PR actually allows the opposite. On that note, maybe it would be nice if |
Yeah, I was thinking about it, but then we'd need to somehow track all the instances of the However, maybe we should not expose any API at all? What if the iteration on the internal
|
|
On Mon, 01 Apr 2024 13:36:08 +0200, Wojciech Mazur wrote:
However, maybe we should not expose any API at all? What if the iteration on the internal `QueueExecutionContext` would happen automatically in some well known parts of Java standard library, eg. `Thread.sleep`, `Thread.join`, `LockSupport.park`, `Object.wait` - call to any of this gives an opportunity to iterate over the queue execution context.
`LockSupport.park` is used to implement `Await.result/ready` so it would be enough to cover the basic needs
Can you drop an example to implement a scala-native-loop on that?
…--
wbr, Kirill
|
|
To be clear: https://github.com/scala-native/scala-native-loop is quite official and release the next version of scala-native which brokes it, without any way to fix it is insane, isn't it? |
This was actually one of the use cases which I think would be great. This would effectively work as a better
I don't have a strong opinion about this, although, for reference, I have some code that relies on |
Move to scalanative.concurrent package
|
I've prepared a next iteration of the API, 2 main additions:
|
@JD557 Any library that is currently hard-coding This is precisely my point :) I'm not convinced that anybody actually needs these "stealing" APIs unless they are hard-coding an There are really two kinds of libraries we are talking about:
When wiring up an application, the user just needs to pass the EC from (2) to (1).
@catap I am confident there is a way to fix it. Again, I strongly encourage you to look at Cats Effect where we have already implemented a working event loop without these APIs and without any hacks. I don't know what else to say, it really is possible. @WojciechMazur I appreciate the iteration and I'll take a look, but based on this discussion I've become less convinced that we need this API at all, unless we are deliberately supporting the hard-coding usecase. If we insist that we are not supporting that usecase, then I no longer see the motivation for these changes. |
I need a layer which allows to use the same code in JVM, Native and JS for both DOM and Node. I try to avoid here any dependencies and frankly speaking it has a few dependencies. Adding Cats Effects seems a bit overkill here. Special that code base is stable enough and I don't see any reason to move it to something else, frankly speaking I better to make one more RawPtr hack, without touching it again to stabelize it. Thus, the right move is implementing NIO API inside SN and SJS, but as far as I recall it won't happening for the last project, so, I stuck with may wrapper anyway. |
|
When it comes to hard-coded execution contexts in the core Scala Native repo there is only 1 usage of them - testing interface. When running tests we use either ForkJoinPool based execution context when multithreading is enabled, or queue execution context when running in single threaded mode. Because we're spawning a new process for each native test runner we need to support this hard coded queue. |
Frankly speaking I really think that it should be backported to upstream Scala. |
Yes, that's right. Fortunately this is not a problem in practice, because these test tasks happen at the "end of the world" i.e. they do not need to be interleaved with other user tasks submitted during testing. This is why we could integrate successfully in Cats Effect.
I still have not seen a good reason that this needs to be a public API.
In my opinion, this safety hatch is not necessary and it encourages poor library design that hard-code the Just for the record, can anyone here provide an example of a library that does that, and why it cannot be fixed to take an I'm sorry, I don't mean to be obstructive: I just want us to be confident that there is a real-world motivation for making this change. Implementing event loops is an advanced topic and I am concerned that this API is being added out of confusion instead of because it is necessary. |
I am tending to to side with the passing of an execution context. I think this is different in kind to what Scala Native has to do internally to manage single and multi-threaded. Although I am certainly not an expert in this area, it seems to be the approach that library creators should use and that is what is taught to users as well. I do also agree with @catap that changes that apply equally to all three platforms should be implemented upstream where that makes sense. |
Based on discussion from #3144 about new api for NativeExecutionContext that shall be maintanble under the changes to Scala standard library in the future.
Currently we'd like to add only work-stealing part so threads can help with completion of tasks from the
QueueExecutionContext, it's especially important in the context of #3852 where we can have multiple task producers.I'm not sure if we should add scheduling capability at this point, probably this can be added in a separate PR
NativeExecutionContext.queueruntime.loop, instead useNativeExecutionContext.queue.helpComplete