Fixes#3262 by adding the missing APIs.
### Motivation:
As explained by the issue linked above, having a
non-`Sendable`-requiring variant of the new `scheduleCallback` APIs on
`NIOIsolatedEventLoop` can be useful since we're not always dealing with
`Sendable` types.
### Modifications:
This adds the required `scheduleCallback` APIs to `NIOIsolatedEventLoop`
by wrapping the non-`Sendable` `NIOScheduleCallbackHandler` in a
`NIOLoopBound`-based handler.
### Result:
The two `scheduleCallback`s can be used on `NIOIsolatedEventLoop` too.
---------
Co-authored-by: Fabian Fett <fabianfett@apple.com>
Motivation:
Public static lets can serve a bunch of roles, but one of them is to
store simple constants: integers, and other trivial types, for example.
This is a nice pattern and for internal and private static lets it works
well, but for public ones it produces some inefficient code.
In particular, it has two downsides. First, it allocates storage for
that value. We don't actually need to allocate a few hundred extra
megabytes for the various integers we want to store.
Secondly, it forces calling code to access the address and call the
dispatch_once code in order to get hold of the value. For trivial types
we don't need that cost: they can just know what the value is directly.
Inlinable computed vars avoid all of these costs: they have no size
overhead for storage, and they are visible to all clients so their
values can be directly assembled.
While I'm here, I added a bunch of other inlinable annotations for a few
trivial data types I stumbled onto.
Modifications:
- Added loads of inlinables. Like, loads.
- Swapped many static lets to static vars.
Result:
Better codegen, smaller memory footprints, more attributes.
### Motivation:
Whilst working on the NIOHTTP1 strict concurrency work I encountered a
few gaps in the API where I needed isolated analogues of methods on
futures.
### Modifications:
* Add `makeSucceededIsolatedFuture`
* Add a new internal initializer for `EventLoopFuture` which does not
require the value to be Sendable for use in isolated contexts.
* Add a variant of `flatMapError` which can handle closures which return
isolated futures
* Add a `futureResult` computed property which allows us to avoid the
otherwise clumsy `future.nonIsolated().futureResult.assumeIsolated()`
### Result:
More ability to work with isolated futures.
Motivation:
As pointed out in #3071, the `flatScheduleTask` implementations can be
improved.
Modifications:
- Refactor the `flatScheduleTask` implementations to skip `flatMap`
calls, which avoids creating an extra promise.
- As there is now a lower number of allocations, reduce the necessary
thresholds for the allocation tests.
Result:
Reduction in the number of allocations in the package.
---------
Co-authored-by: George Barnett <gbarnett@apple.com>
Motivation
Right now the isolated view on the EventLoop has to implement its
interface in terms of UnsafeTransfer, because there are no non-Sendable
operations it can use.
That hurts us because we have to allocate an extra closure for all these
operations, making them slower than they need to be. This is unavoidable
in some contexts, but most EventLoops would be able to offer a fast-path
that avoids this extra allocation.
To achieve that, we need to move this logic into a customisation point
on the EventLoop protocol. Our fallback default implementation can be
the same as we do today.
Modifications
- Add customisation points for EventLoop for each of its operations that
has a protocol witness already.
- Add default implementations of these, using the implementation from
EventLoop.Isolated.
Results
No behavioural changes, but the code has moved.
---------
Co-authored-by: George Barnett <gbarnett@apple.com>
Adds `TimeAmount.init(string:)` and `TimeAmount.description` for parsing
time amounts from strings and pretty printing them.
Closes#2504.
### Motivation:
Had a minute, wanted to work on Swift-NIO a bit more, and saw @weissi
still wanted this.
### Modifications:
It's largely based on the snippet @weissi made in #2504 with a few
changes:
- Added unit tests.
- Added support for more unit aliases for convenience based on
@glbrntt's suggestion.
- Changed `.gitignore` to ignore `.build` everywhere, I've got a few of
them when testing locally.
### Open questions:
- I originally thought perhaps I should add support for multiple number
and unit pairs, i.e. `1h 31m`, but decided against it. Feels like an
edge case. Happy to add this if you think it's needed.
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
Swift 6.1 nightlies have added many new warnings and errors to NIO. This
patch resolves all of them.
Modifications:
- Clean up missing imports
- Move away from @_implementationOnly where necessary
- Fix a few things where Sendable mismatches were present.
Result:
Clean compile again
---------
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
### Motivation
Code that tries to work with `NIODeadline` can be challenging to test
using `EmbeddedEventLoop` and `NIOAsyncTestingEventLoop` because they
have their own, fake clock.
These testing event loops do implement `scheduleTask(in:_)` to submit
work in the future, relative to the event loop clock, but there is no
way to get the event loop's current notion of "now", as a `NIODeadline`,
and users sometimes find that `NIODeadline.now`, which returns the time
of the real clock, to be surprising.
### Modifications
Add `EventLoop.now` to get the current time of the event loop clock.
### Result
New APIs to support writing code that's easier to test.
### Motivation:
`NIOEventLoopGroupProvider.createNew` was probably never a good idea
because it creates shutdown issues for any library that uses it. Given
that we now have singleton (#2471) `EventLoopGroup`s, we can solve this
issue by just not having event loop group providers.
Users can just use `group: any EventLoopGroup` and optionally `group:
any EventLoopGroup = MultiThreadedEventLoopGroup.singleton`.
### Modifications:
- deprecate `NIOEventLoopGroupProvider.createNew`
- soft-deprecate (document as deprecated but don't mark
`@available(deprecated)`) `NIOEventLoopGroupProvider`
### Result:
- Libraries becomes easier to write and maintain.
- Fixes#2142
### Motivation:
Documentation checking catches more issues in Swift 6.0.
### Modifications:
Adopt the Swift 6.0 image and fix the errors.
### Result:
More accurate docs.
# Motivation
We need to tackle the remaining strict concurrency checking related
`Sendable` warnings in NIO. The first place to start is making sure that
`EventLoopFuture` and `EventLoopPromise` are properly annotated.
# Modification
In a previous https://github.com/apple/swift-nio/pull/2496, @weissi
changed the `@unchecked Sendable` conformances of
`EventLoopFuture/Promise` to be conditional on the sendability of the
generic `Value` type. After having looked at all the APIs on the future
and promise types as well as reading the latest Concurrency evolution
proposals, specifically the [Region based
Isolation](https://github.com/apple/swift-evolution/blob/main/proposals/0414-region-based-isolation.md),
I came to the conclusion that the previous `@unchecked Sendable`
annotations were correct. The reasoning for this is:
1. An `EventLoopPromise` and `EventLoopFuture` pair are tied to a
specific `EventLoop`
2. An `EventLoop` represents an isolation region and values tied to its
isolation are not allowed to be shared outside of it unless they are
disconnected from the region
3. The `value` used to succeed a promise often come from outside the
isolation domain of the `EventLoop` hence they must be transferred into
the promise.
4. The isolation region of the event loop is enforced through
`@Sendable` annotations on all closures that receive the value in some
kind of transformation e.g. `map()` or `whenComplete()`
5. Any method on `EventLoopFuture` that combines itself with another
future must require `Sendable` of the other futures `Value` since we
cannot statically enforce that futures are bound to the same event loop
i.e. to the same isolation domain
Due to the above rules, this PR adds back the `@unchecked Sendable`
conformances to both types. Furthermore, this PR revisits every single
method on `EventLoopPromise/Future` and adds missing `Sendable` and
`@Sendable` annotation where necessary to uphold the above rules. A few
important things to call out:
- Since `transferring` is currently not available this PR requires a
`Sendable` conformance for some methods on `EventLoopPromise/Future`
that should rather take a `transffering` argument
- To enable the common case where a value from the same event loop is
used to succeed a promise I added two additional methods that take a
`eventLoopBoundResult` and enforce dynamic isolation checking. We might
have to do this for more methods once we adopt those changes in other
targets/packages.
# Result
After this PR has landed our lowest level building block should be
inline with what the rest of the language enforces in Concurrency. The
`EventLoopFuture.swift` produces no more warnings under strict
concurrency checking on the latest 5.10 snapshots.
---------
Co-authored-by: George Barnett <gbarnett@apple.com>
Co-authored-by: Cory Benfield <lukasa@apple.com>
# Motivation
We only support the last three Swift released versions which are at this
time 5.9, 5.10 and 6.
# Modification
This PR drops anything related to Swift 5.8.
# Result
Version support aligned.
## Motivation
The current `scheduleTask` APIs make use of _both_ callbacks and
promises, which leads to confusing semantics. For example, on
cancellation, users are notified in two ways: once via the promise and
once via the callback. Additionally the way the API is structured
results in unavoidable allocations—for the closures and the
promise—which could be avoided if we structured the API differently.
## Modifications
This PR introduces new protocol requirements on `EventLoop`:
```swift
protocol EventLoop {
// ...
@discardableResult
func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback
@discardableResult
func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback
func cancelScheduledCallback(_ scheduledCallback: NIOScheduledCallback)
}
```
Default implementations have been provided that call through to
`EventLoop.scheduleTask(in:_:)` to not break existing `EventLoop`
implementations, although this implementation will be (at least) as slow
as using `scheduleTask(in:_:)` directly.
The API is structured to allow for `EventLoop` implementations to
provide a custom implementation, as an optimization point and this PR
provides a custom implementation for `SelectableEventLoop`, so that
`MultiThreadedEventLoopGroup` can benefit from a faster implementation.
Finally, this PR adds benchmarks to measure the performance of setting a
simple timer using both `scheduleTask(in:_:)` and
`scheduleCallback(in:_:)` APIs using a `MultiThreadedEventLoopGroup`.
## Result
A simpler and more coherent API surface.
There is also a small performance benefit for heavy users of this API,
e.g. protocols that make extensive use of timers: when using MTELG to
repeatedly set a timer with the same handler, switching from
`scheduleTask(in:_:)` to `scheduleCallback(in:_:)` reduces almost all
allocations (and amortizes to zero allocations) and is ~twice as fast.
```
MTELG.scheduleCallback(in:_:)
╒═══════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric │ p0 │ p25 │ p50 │ p75 │ p90 │ p99 │ p100 │ Samples │
╞═══════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Malloc (total) * │ 0 │ 0 │ 0 │ 0 │ 0 │ 0 │ 0 │ 1109 │
╘═══════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛
MTELG.scheduleTask(in:_:)
╒═══════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric │ p0 │ p25 │ p50 │ p75 │ p90 │ p99 │ p100 │ Samples │
╞═══════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Malloc (total) * │ 4 │ 4 │ 4 │ 4 │ 4 │ 4 │ 4 │ 576 │
╘═══════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛
```
Dispatch is not supported on WASI, and only Unix domain sockets are
supported, which means we have to exclude those APIs on this platform.
There's work in progress to enable tests for this on CI, but nothing I
can provide for this PR at the current moment.
---------
Co-authored-by: Franz Busch <f.busch@apple.com>
* Apply formatting
* Apply no block comments rule
* Apply OmitExplicitReturns
* Apple OnlyOneTrailingClosureArgument
* Apply NoAssignmentInExpressions
* Fix up DontRepeatTypeInStaticProperties lint errors
* Apply `OrderedImports`
* Apply `ReplaceForEachWithForLoop`
* format file
* Enable the formatting pipeline
* Adopt `AmbiguousTrailingClosureOverload`
* Fix license header
* Fix format check
* Fix `EndOfLineComment`
* Fix CI
* Adapt CI script to check if changes when running formatting
* Separate lint and format into to steps
* Fix format
* Adopt `UseEarlyExits`
* Revert "Adopt `UseEarlyExits`"
This reverts commit d1ac5bbe12.
Motivation:
Existential errors (`any Error`) are unconditionally boxed. In NIO we
often create errors to fail promises. One place we do this is in the
head channel handler when the pipeline is closed. This error is created
regardless of whether there are actually any promises to fail. The
result is an unnecessary allocation when every channel is closed. This
is particuarly egregious when multiplexed protocols such as HTTP/2 are
used.
As the majority of errors in NIO carry no information beyond their
type/name we can declare these statically and pay the cost of each error
just once rather than once per use.
Modifications:
- Add internal static constants for `ChannelError` and `EventLoopError`
and use them throughout NIOCore and NIOPosix where they would
otherwise be boxed.
Result:
Fewer allocations
# Motivation
Next up replacing the documentation check.
# Modification
This moves the current approach of the script into a GH action. I tried just running the documentation check against all targets but that also results in warnings from downstream deps in their docs to show up.
I also fixed up some new errors that appeared since our current pipeline was running on 5.8.
# Result
Next CI job migrated to GHA
# Motivation
We have a few more warnings left under strict concurrency checking in `NIOCore`. This fixes the bulk of them that are not tied to `ChannelHandler` or `ChannelPipeline`.
# Modification
Add more `Sendable` and `@Sendable` annotations and make sure to use types that are `Sendable` annotated.
# Motivation
Currently, the NIO's EventLoop conformance to the `SerialExecutor` protocol always uses `execute` to schedule the actual job. However, the closure for `execute` has to close over the job and the `EventLoop` itself; hence, it always allocates. Since jobs are a very fine grained object in Concurrency that are created a lot this lead to millions of allocations in even small benchmarks.
# Modification
This PR provides a customization point for `EventLoop`s to execute `ExecutorJob`s directly. For `SelectableEventLoop` we store a type erased `UnownedJob` in our `ScheduledTask` and just run it right away.
# Result
No more allocations when NIO's EL is used as a `SerialExecutor`.
* Bump minimum Swift version to 5.7
Motivation:
Now that Swift 5.9 is GM we should update the supported versions and
remove 5.6
Modifications:
* Update `Package.swift`
* Remove `#if swift(>=5.7)` guards
* Delete the 5.6 docker compose file and make a 5.10 one
* Update integration test script
* Update docs
Result:
Remove support for Swift 5.6, add 5.10
* fix indentation issues
* 5.9 docker image use release image
* Support NIO Event Loops as actor executors
Motivation
Swift Concurrency supports having actors opt in to executing
within a "custom executor". This requires being able to be strictly
serial, a constraint that NIO's EventLoops natively meet. It is
useful to have actors be able to express a "tie" to an EventLoop
in order to efficiently access objects that are tied to specific ELs.
Modifications
- Extend EventLoop to expose an executor property
- Provide a simple best-effort default using NIODefaultSerialEventLoopExecutor
- Expose NIOSerialEventLoopExecutor protocol that makes it easy
for adopters to get a better implementation
- Crash if EmbeddedEventLoop is used
- Add tests
Result
Adopters can use ELs as serial executors
* Prefer #if compiler
mark syncShutdownGracefully noasync
Motivation:
The code as-is blocks the calling thread.
Modifications:
* mark `EventLoopGroup.syncShutdownGracefully()` and `NIOThreadPool.syncShutdownGracefully()` noasync on Swift > 5.7
* offer NIOThreadPool.shutdownGracefully()
* add renamed to syncShutdownGracefully()
Motivation:
Our time types are trivial, and they should be fully transparent. This
produces minor performance improvements in code handling time types, but
is mostly useful in terms of allowing the compiler to observe that these
functions have no side effects, thereby eliding some ARC traffic.
Modifications:
Make our time types inlinable.
Result:
Better performance.
Motivation:
#fileID introduced in Swift 5.3, so no longer need to use #file anywhere
Modifications:
Changed #file to #filePath or #fileID depending on the situation
* Adopt `Sendable` in `EventLoop.swift`
* only adopt `Sendable` in Swift 5.6+
* add `@Sendable` only for Swift 5.7
* fix swift 5.5
* use internal typealias to deduplicate method bodies
* wip: Use clock_gettime for NIODeadline.now()
Signed-off-by: Si Beaumont <beaumont@apple.com>
* fixup: Add #if os(Linux) for clock_gettime use
* fixup: Add doc comments
Signed-off-by: Si Beaumont <beaumont@apple.com>
Motivation
The rise of Swift concurrency has meant that a number of our APIs need
to be recontextualised as async/await capable. While generally this is a
straightforward task, any time those APIs were tested using
EmbeddedChannel we have a testing issue. Swift Concurrency requires the
use of its own cooperative thread pool, which is completely incapable of
safely interoperating with EmbeddedChannel and EmbeddedEventLoop. This
is becuase those two types "embed" into the current thread and are not
thread-safe, but our concurrency-focused APIs want to enable users to
use them from any Task.
To that end we need to develop new types that serve the needs of
EmbeddedChannel and EmbeddedEventLoop (control over I/O and task
scheduling) while remaining fully thread-safe. This is the first of a
series of patches that adds this functionality, starting with the
AsyncEmbeddedEventLoop.
Modifications
- Define AsyncEmbeddedEventLoop
Result
A required building block for AsyncEmbeddedChannel exists.
Co-authored-by: Franz Busch <privat@franz-busch.de>
### Motivation:
In my previous PR https://github.com/apple/swift-nio/pull/2010, I was able to decrease the allocations for both `scheduleTask` and `execute` by 1 already. Gladly, there are no more allocations left to remove from `execute` now; however, `scheduleTask` still provides a couple of allocations that we can try to get rid of.
### Modifications:
This PR removes two allocations inside `Scheduled` where we were using the passed in `EventLoopPromise` to call the `cancellationTask` once the `EventLoopFuture` of the promise fails. This requires two allocations inside `whenFailure` and inside `_whenComplete`. However, since we are passing the `cancellationTask` to `Scheduled` anyhow and `Scheduled` is also the one that is failing the promise from the `cancel()` method. We can just go ahead and store the `cancellationTask` inside `Scheduled` and call it from the `cancel()` method directly instead of going through the future.
Importantly, here is that the `cancellationTask` is not allowed to retain the `ScheduledTask.task` otherwise we would change the semantics and retain the `ScheduledTask.task` longer than necessary. My previous PR https://github.com/apple/swift-nio/pull/2010, already implemented the work to get rid of the retain from the `cancellationTask` closure. So we are good to go ahead and store the `cancellationTask` inside `Scheduled` now
### Result:
`scheduleTask` requires two fewer allocations
Motivation:
For NIO's 'promise leak detector' we added file:line: labels to
makePromise and there it makes sense. A user might create a promise and
then never fulfil it, bad. With the file:line: arguments we can give
good diagnostics.
However, we (probably that @weissi again) also added it to flatMap and
friends where it doesn't make sense at all.
Sure, EventLoopFuture's implementation may create a promise in the
implementation of flatMap but this promise is never leaked unless the
previous future is never fulfilled (or NIO has a terrible bug). Suffice
to say that in a future chain, it's never a flatMap etc which is
responsible for leaking the first promise...
Explain here the context, and why you're making that change.
What is the problem you're trying to solve.
Modifications:
Remove all unnecessary `file:line:` parameters whilst keeping the public
API intact.
Result:
More sensible code.
Motivation:
A lot of libraries that use SwiftNIO don't allow/require the user to
specify what `EventLoop`s a certain function runs on. Something like
```
myHTTPClient.get("https://example.com) -> EventLoopFuture<...>
```
Internally `MyHTTPClient` has not much choice but using the
`EventLoopGroup.next()` method to obtain an `EventLoop` on which to
create the `EventLoopFuture` (that is returned).
This all works fine but unfortunately it usually forces a thread switch
which is most of the time avoidable if we're already running on an
`EventLoop`.
Modifications:
Provide an `EventLoopGroup.any()` method which can be used like so:
```swift
func get(_ url: String) -> EventLoopFuture<Response> {
let promise = self.group.any().makePromise(of: Response.self)
[...]
return promise.futureResult
}
```
`EventLoopGroup.any()` very much works like `EventLoopGroup.next()`
except that it tries -- if possible -- to return the _current_
`EventLoop`.
Note that this means that `any()` is _not_ the right solution if you
want to load balance. Likely, everything will now stay on the same
`EventLoop`.
Result:
Fewer thread switches.
Motivation:
The next step on extracting the base abstractions is to pull out
EventLoop, EventLoopGroup and related types, and also EventLoopPromise
and EventLoopFuture. These types are fundamental to the API.
Modifications:
- Extract EventLoop, EventLoopGroup, and related types.
- Extract EventLoopPromise and EventLoopFuture.
- Add new @testable imports.
- Split MultiThreadedEventLoopGroup into new file, appropriately named.
Result:
Better separation.
Paul ToffoloniandFabian Fett
Cory Benfield
Johannes Weiss
Rick Newton-Rogers
Clinton NkwochaandGeorge Barnett
Natik GadzhiandCory Benfield
Si Beaumont
Max DesiatovandFranz Busch
Lorenzo Fritzsch
Rick Newton-Rogers
David Nadoba
carolinacass
BenedictSt