Motivation:
The isolated execute on async testing event loop wouldn't execute tasks
in the case of promise being nil
Modifications:
Rewrite the execute logic slightly.
Result:
Correct execution
Co-authored-by: George Barnett <gbarnett@apple.com>
Motivation:
NIOAsyncTestingEL is another place where we can reduce the overhead of
the isolated EL views. It's nice for the tests to run faster.
Modifications:
- Implement the new protocol witnesses for AsyncTestingEL
- Refactor common code for scheduleTask out
Result:
AsyncTestingEL is faster.
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:
NIOEmbedded is used all over NIO-land for testing various pieces of the
infrastructure, and so requires a substantial audit for strict
concurrency.
Modifications:
- Mark a few things Sendable.
- Fix the tests, which actually did have some nasty bugs
Result:
Sendable-clean NIOEmbedded
### 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
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>
### Motivation:
The two testing event loops—`EmbeddedEventLoop` and `NIOAsyncTestingEventLoop`—have different semantics for outstanding work during shutdown, which are both different from the production `SelectableEventLoop`, specifically with newly scheduled tasks that result from running existing scheduled work at the time of shutdown.
There are three axes to consider:
1. Scheduled tasks that are at or past their deadline.
2. Scheduled tasks with a deadline in the future.
3. Newly scheduled work that result from either running or cancelling (1) and (2).
| | (1) | (2) | (3) |
|-|-|-|-|
| `SelectableEventLoop` | Run | Cancel | Quiesce over 1000 ticks, repeatedly run resulting (1) and cancel resulting (2) |
| `EmbeddedEventLoop` | Run | Cancel | Cancel resulting (1) and resulting (2) |
| `NIOAsyncTestingEventLoop` | Run | Run | Run resulting (1) and resulting (2) |
Note that `NIOAsyncTestingEventLoop` may never terminate because of this.
### Modifications:
This PR aligns `EmbeddedEventLoop` and `NIOTestingEventLoop` and makes them more similar to `SelectableEventLoop` and less surprising semantics.
### Result:
| | (1) | (2) | (3) |
|-|-|-|-|
| `SelectableEventLoop` | Run | Cancel | Quiesce over 1000 ticks, repeatedly run resulting (1) and cancel resulting (2) |
| `EmbeddedEventLoop` | Run | Cancel | Cancel resulting (1) and resulting (2) |
| `NIOAsyncTestingEventLoop` | Run | Cancel | Cancel resulting (1) and resulting (2) |
### Future work:
Given both the `EmbeddedEventLoop` and `NIOAsyncTestingEventLoop` are used in tests of NIO applications that run with `SelectableEventLoop` in production, it might be worth extending both to also support quiescing to give a more representative shutdown behaviour in tests.
* 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.
* 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
* Enhance and rename AsyncEmbeddedEventLoop
Motivation
AsyncEmbeddedEventLoop is an important part of our ongoing testing story
for NIO. However, it suffers from two problems.
The first is a usability one. As we discovered during the original
implementation, following EmbeddedEventLoop's pattern of not having any
EventLoop "Tasks" execute until run() meant that simple constructs like
`EventLoopFuture.wait` and `EventLoopFuture.get` didn't work at all,
forcing us to add an annoying `awaitFuture` method.
When playing with implementing EmbeddedChannel, this got worse, as I/O
methods also don't run immediately if called from the testing thread. We
couldn't easily work around this issue, and it meant that common
patterns (like calling `channel.writeAndFlush`) would deadlock!
This is unacceptable, so a change had to be made.
While we're here, we received feedback that the name is unclear to
users. Given that this particular event loop is in no sense "embedded",
we no longer need the name, so we can take this opportunity to use a
better one.
Modifications
Changed `func execute` to immediately execute its task body, and to
dequeue all pending tasks at this time. Essentially, it's the equivalent
to run(). This is a major change in its behaviour.
Renamed the loop to `NIOAsyncTestingEventLoop`.
Result
Better names, easier to use.
* Make soundness happy
* Remove awaitFuture
Cory Benfield
Si Beaumont
Franz Busch
Max Desiatov
David Nadoba
Johannes Weiss