Skip to content

use NIOSingletons EventLoops/NIOThreadPool instead of spawning new #697

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.

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

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

weissi
Copy link
Contributor

@weissi weissi commented Jul 13, 2023

@weissi weissi requested review from dnadoba and fabianfett July 13, 2023 06:15
@weissi weissi force-pushed the jw-nio-singletons branch 2 times, most recently from bf21183 to eadd7e6 Compare July 13, 2023 08:49
weissi added a commit to apple/swift-nio that referenced this pull request Jul 27, 2023
### Motivation:

SwiftNIO allows and encourages to precisely manage all operating system resources like file descriptors & threads. That is a very important property of the system but in many places -- especially since Swift Concurrency arrived -- many simpler SwiftNIO programs only require a single, globally shared EventLoopGroup. Often even with just one thread.

Long story short: Many, probably most users would happily trade precise control over the threads for not having to pass around `EventLoopGroup`s. Today, many of those users resort to creating (and often leaking) threads because it's simpler. Adding a `.globalSingle` static var which _lazily_ provides singleton `EventLoopGroup`s and `NIOThreadPool`s is IMHO a much better answer.

Finally, this aligns SwiftNIO a little more with Apple's SDKs which have a lot of global singletons that hold onto system resources (`Dispatch`'s thread pool, `URLSession.shared`, ...). At least in `Dispatch`'s case the Apple SDKs actually make it impossible to manage the resources, there can only ever be one global pool of threads. That's fine for app development but wouldn't be good enough for certain server use cases, therefore I propose to add `NIOSingleton`s as an _option_ to the user. Confident programmers (especially in libraries) are still free and encouraged to manage all the resources deterministically and explicitly.

Companion PRs: 
 - apple/swift-nio-transport-services#180
 - swift-server/async-http-client#697

### Modifications:

- Add the `NIOSingletons` type
- Add `MultiThreadedEventLoopGroup.singleton`
- Add `NIOThreadPool.singleton`

### Result:

- Easier use of NIO that requires fewer parameters for users who don't require full control.
- Helps with #2142
- Fixes #2472
- Partially addresses #2473
@weissi weissi force-pushed the jw-nio-singletons branch from eadd7e6 to 4c2c2a7 Compare July 27, 2023 14:06
@weissi weissi requested review from FranzBusch and glbrntt July 27, 2023 14:06
@weissi weissi force-pushed the jw-nio-singletons branch from 4c2c2a7 to 2c0b42e Compare July 27, 2023 14:09
@weissi
Copy link
Contributor Author

weissi commented Aug 12, 2023

@FranzBusch @glbrntt @dnadoba This is ready for review now.

@weissi weissi force-pushed the jw-nio-singletons branch 2 times, most recently from 2ff11f5 to 0c1d00d Compare August 12, 2023 18:41
```swift
import AsyncHTTPClient

let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
let httpClient = HTTPClient(eventLoopGroupProvider: .singleton)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason these examples don't use the zero-element initializer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lukasa Yes, that's only coming in #705 . #705 does also rewrite all these examples to just use HTTPClient.shared. But I split this work into two parts (maybe misguidedly):

  1. Introduce the .singleton EventLoopGroupProvider --> this PR
  2. Introduce HTTPClient.shared, a completely shared singleton HTTPClient that doesn't need shutdown --> #700 & #701: HTTPClient.shared a globally shared singleton #705

@weissi weissi requested a review from Lukasa August 14, 2023 10:42
@weissi weissi force-pushed the jw-nio-singletons branch from 0c1d00d to e720e1c Compare August 14, 2023 10:42
@weissi weissi requested a review from glbrntt August 14, 2023 10:42
@weissi weissi merged commit 8c90405 into swift-server:main Aug 14, 2023
@weissi weissi deleted the jw-nio-singletons branch August 14, 2023 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants