Closed weissi closed 1 week ago
Hmm, I agree that the pattern is ugly, but do we think it rises to "really bad" in a way that justifies having a singleton event loop group?
Hmm, I agree that the pattern is ugly, but do we think it rises to "really bad" in a way that justifies having a singleton event loop group?
I hate global singletons with a passion. But:
So I think the prior art from the Apple SDKs and the lack of real alternatives lead to me believe that it's the least of all the evils. Any better ideas?
I mean, the simplest idea is the one that we have been falling back to all along: creating the ELG is the app's business, not the business of any library. This is coherent (apps have lifecycles, libraries don't) and customizable. It's also easier to manage and test.
I somewhat agree that the EventLoopGroupProvider
is problematic; however, I am leaning towards what @Lukasa is suggesting here that libs should never create/own ELGs and this is up to the application to provide.
I mean, the simplest idea is the one that we have been falling back to all along: creating the ELG is the app's business, not the business of any library. This is coherent (apps have lifecycles, libraries don't) and customizable. It's also easier to manage and test.
To chime in here - in the world where Swift Concurrency is everywhere and libraries like Vapor, AHC etc don't expose or have ELGs in their public interfaces, forcing it to be passed around is confusing for an end user, which I think is the point @weissi was making. Is this something that goes solved eventually with custom executors?
I mean, the simplest idea is the one that we have been falling back to all along: creating the ELG is the app's business, not the business of any library.
Ideologically, I agree with you. Practically, I don't. People just don't want to carry random amount of stuff through their whole application. That's also why we created the ELGProvider pattern in AHC, it was just not acceptable to force everybody to import NIO
and create an ELG. Especially given that the correct type of ELG also somewhat depends on your OS (NIO on Sockets vs. NIOTS).
This is coherent (apps have lifecycles, libraries don't) and customizable. It's also easier to manage and test.
Well, certain objects (like HTTPClient
in AHC) will need lifecycle anyway to get rid of some resources but in general I get your point and again, ideologically agreed.
To chime in here - in the world where Swift Concurrency is everywhere and libraries like Vapor, AHC etc don't expose or have ELGs in their public interfaces, forcing it to be passed around is confusing for an end user, which I think is the point @weissi was making.
Exactly. I think Vapor did this really neatly by just (by default) owning the ELG in the App. And because Vapor's a framework that works just fine. You create your Vapor.Application
in main and then you also own an ELG.
AsyncHTTPClient and other libraries (that aren't the application framework) this is much harder because they shouldn't [resources]/can't [lifecycle] all own their ELGs for every HTTPClient
.
In this issue, I'd like to discuss the use case for libraries, i.e. things that might be used as an implementation detail in some other library/application where it'd be awkward to require the initialisation of something in main that then needs to be passed around.
Right now, this is actually even harder because of issues like
EventLoopGroup
and an HTTPClient
(and probably even more stuff in the future) to everywhere.An implicit event loop group is still going to require someone to choose what the ELG is, or we'll commit a layering violation. NIOCore
does not have an event loop group available to it, so it can't make a default choice for the user. We could let each module attempt to register whatever ELG it provides as the default, but only one of these can win, so if you've imported NIOPosix
and NIOTransportServices
and NIOEmbedded
then you're in real trouble, because each of those provides its own ELG (and NIOEmbedded
provides two). If we're expecting this to work well for users then this conflict must have a deterministic resolution.
Additionally, Swift provides no mechanism for running module constructors so in practice it is entirely possible that none of these modules have executed any code at the point when someone tries to use a default implicit ELG, at which point the only thing we can do is crash (again, NIOCore
can't fix this for you). We could work around that by having libraries perform fallbacks to initialise the default if it's uninitialised at the point of use, but at this point we've largely just reinvented the existing problem.
We could work around that by having each module define its own implicit global event loop group (and then the library just has to choose which of those implicit loops it wants to use), but at that stage we end up with the result that there are several implicit global event loop groups. I'm honestly not sure this solution is better than the problem we currently have.
@Lukasa or we could have a new repo, let's call it swift-nio-family
which has a NIOGlobalExecutor
or so module or whatever which depends on everything and provides an ELG that it thinks is best.
Then AHC and others can depend on just swift-nio-family
and unless the users passes an ELG to share, they'll all just share the NIOGlobalExecutor
one.
WDYT? That'd also keep the singletons out of the core NIO repos.
Yup, we can do that, but then we get two new problems:
I'm very open to better, more open and composable solutions but I do think we need to address the real issue here which can probably be summed up as
There is nothing good that a library can do today.
Why?
EventLoopGroupProvider
has very bad API (forcing lifecycle managements, weird shutdown signature) consequences and just isn't good.createNew
/.global
/something easy then users feel that library is overly complicated.I think my position is that we have no good options. It is not at all clear to me that replacing an ugly cleanup pattern with implicit global state solves more problems than it creates. We'll certainly have different problems, but that's not the same as fewer.
We're fundamentally trying to thread a needle that no-one else has to thread: we want to make it possible to use libraries without any configuration, but enable extensive configuration when users want it, with a distributed set of possible implementations where users may want to avoid having any particular one of them present. That's just gonna be hard. We're probably going to have to decide what problem we think is most important and accept that we're going to rule out a few others.
My position is that we have to do something because it forces this bad API choice on everybody who consumes AHC or other libraries using this pattern.
And fact is that with Swift Concurrency being the future, most of the code is running on a global singleton pool anyway, already.
Where are we in this discussion?
I haven't seen much reference to this discussion in the SSWG meeting notes since June, and the last discussion was had in June.
The NIO ecosystem suffers from one composability issue:
EventLoopGroupProvider
and similar constructs. The issue is that takingEventLoopGroupProvider
(or similar) means that a library can either own or not own anEventLoopGroup
.That doesn't sound too bad but unfortunately, it's very bad for the shutdown case.
A library (like say AsyncHTTPClient) would obviously want to have a method called
thing.shutdown() -> EventLoopFuture<Void>
to match the other functions. Unfortunately, that can't be done because the library may own theEventLoopGroup
, thereforeshutdown()
has to may have to shut it down which then however means that there is no moreEventLoop
left to run the future on.The current hack is to instead provide a
shutdown(group: DispatchGroup, _ completion: @escaping (Error?) -> Void)
method. But that's both ugly (doesn't fit the rest) and wasteful (needs to spin up a dispatch thread for no reason).Unfortunately, this issue won't just magically be fixed by the universal adoption of Swift Concurrency either. Yes, it's possible to provide a
func shutdown() async throws -> Void
but that'll then bounce from a Concurrency executor to the EventLoop to a DispatchQueue thread back to some Concurrency executor. That's still not great.Also, there's another issue if
EventLoop(Group)
s frequently go away: What do we do with people who caught a reference to an EL(G) and then (after the shutdown)execute
stuff on it? Currently we print this weird warning that we'll crash in future releases. That's cool if EL(G)s are mostly global/created in main but it really doesn't work if they're frequently created/destroyed.Also there's a question w.r.t. Custom Executors (once they actually arrive in Swift): Will they support this use case?
My recommendations are:
EventLoopGroupProvider
HTTPClient
) creates its own EL(G) threadsIf we did deprecate
EventLoopGroupProvider
and the whole pattern, what should users do who don't already have an EL(G)? This will also become even more prevalent in the future where every user-facing API will be Swift Concurrency?The only solution I can come up with is for either SwiftNIO itself or every library lazily creating an internal ELG that will never be shut down. In other words: A global EL(G) singleton that gets created on first use, much like Dispatch or Swift Concurrency work. From a resource perspective and also alignment with Swift Concurrency it might actually make sense if SwiftNIO owned that singleton EL(G). That'd mean the new pattern to replace
EventLoopGroupProvider
would be.shared(elg)
or.global
.The alternative to SwiftNIO owning one global ELG would be ofc for every library owning its own global ELG that could maybe just have 1 thread. I.e. AsyncHTTPClient would internally have a singleton ELG with just one thread.
What do you all think about this? None of this is fully baked but I'm really quite sure that the
EventLoopGroupProvider
(and similar) pattern is really bad.