Add Fiber::ExecutionContext::Isolated #15513
Conversation
Introduces the last EC scheduler that runs a single fiber in a single thread. Contrary to the other schedulers, concurrency is disabled. Like the ST scheduler, the scheduler doesn't need to actively park the thread and merely waits on the event loop.
|
|
||
| @wait_list = Crystal::PointerLinkedList(Fiber::PointerLinkedListNode).new | ||
|
|
||
| def initialize(@name : String, @spawn_context = ExecutionContext.default, &@func : ->) |
There was a problem hiding this comment.
thought: I'm wondering whether the default value for spawn_context should be the parent context of the current fiber instead of the application-wide default context?
Given the following code, I believe it would be reasonable to expect fiber bar to spawn in the same context as foo.
spawn name: "foo" { }
ExecutionContext::Isolated.new("iso") do
spawn name: "bar" { }
endThere was a problem hiding this comment.
That makes sense, though it might get a bit confusing.
An alternative could be to not have a spawn context by default, so you must explicitly pass one, or explicitly spawn into a context. That could be a good mean to detect rogue spawns 🤔
There was a problem hiding this comment.
If we allow not having a default context set, calls to ::spawn would raise I suppose? That would be quite poor though when the program compiles but fails at runtime.
Having to explicitly specify the context is also not very user friendly...
I suppose we could start with that though and defer the discussion about default assignment. This topic would also be relevant in the context of structured concurrency.
There was a problem hiding this comment.
Yes, let's start harsh then ease the pain (if any).
There was a problem hiding this comment.
Changed to not have a default, so we must choose what behavior to adopt at compile time, and not discover about this when we get a RuntimeError at runtime, or having fibers silently spawn in some other context.
There was a problem hiding this comment.
Hum, I believe it does. For example the reader loop thread for win32: it doesn't and musn't spawn anything, anywhere.
There was a problem hiding this comment.
I'd even make nil the default. The isolated context is bubble wrap over a raw thread with no concurrency, and of course runtime checks.
Since there are use-cases where we need to spawn (for example execute an async action from a GUI event) the app can spawn into an explicit context, but it might be tedious to always reach for that context, so you can specify a default spawn context.
But IMO that's optional, and mostly a nice touch.
There was a problem hiding this comment.
It doesn't feel like this is something that we must restrict in the execution context though.
The reader thread is an internal tool for the runtime environment. I'm sure we can control its abilities without locking down spawn explicitly.
There was a problem hiding this comment.
It might be an internal tool, it's still a use case.
There was a problem hiding this comment.
But it's not necessarily a use case that we should/need to support in the public API.
The point is to avoid having fiber being spawn in an unknown context from the isolated fiber. While we could default to `nil` it would delay the understanding of the behavior to a runtime error, while having a compilation error will bring up the issue sooner, and makes the intent more explicit. We'll decide later if we want to have a default value, and what (nil? current context? default context?)
Aka bubble wrap a thread.
The
Fiber::ExecutionContext::Isolatedcontext is a special scheduler: it starts a single thread to execute a block a code inside its main fiber. Concurrency is disabled, no other fiber can run on this thread. The fiber owns the thread and can do any number of blocking operations as it needs to without blocking any other fiber.But you can still:
Channel,MutexorWaitGroup);spawnfibers, they will be spawned into another context (for example the default context).NOTE: this PR alone isn't enough to enable execution contexts. It misses either the ST or MT context.
refs #15342