### Motivation:
Currently if there is an error in the Benchmark targets, that prevents
the Benchmark to build correctly, we still get a CI success.
### Modifications:
- Check for the threshold changed error
### Result:
- More reliable CI
Motivation:
I'm unhappy with AddressedEnvelope.Metadata, and I plan to change it.
Before I change it, I want some way to get a clear picture of whether my
changes make things better or worse.
Modifications:
Added a bunch of benchmarks
Result:
Some data will be available.
---------
Co-authored-by: George Barnett <gbarnett@apple.com>
Motivation:
TCPEcho has some run-to-run variation which makes CI flaky.
Modifications:
- Add a little slack to TCPEcho; it has a low total alloc count so some
slack is fine as it's less than a per iteration allocation
Result:
CI less flaky
### Motivation:
`NIOAsyncTestingChannel` stored its `localAddress` and `remoteAddress`
in a locked storage on itself for thread safety, however in doing so
left us open to bugs because a handler grabbing the addresses of the
context had no visibility of the values.
### Modifications:
Reach into `EmbeddedChannelCore` for the addresses instead of storing
them on the `NIOAsyncTestinghannel`. I also considered a delegate
approach where the `EmbeddedChannelCore` could offload the
responsibility for storing the values back to the
`NIOAsyncTestingChannel` but it was complicated and of questionable
value.
### Result:
* The correct address values are seen no matter how they are obtained.
* We probably take a performance hit locking the values in this way but
this is testing code so probably not the end of the world.
Motivation
With the introduction of isolated conformances, it has become necessary
to start managing the use of metatypes for some of our protocols. In
general, we don't want to force the relevant protocols to only be
conformed in non-isolated forms. Instead, we just want to make the
specific APIs non-usable.
Modifications
- Add shims for SendableMetatype that only use it when it is available.
- Require SendableMetatype where needed, gated by @preconcurrency.
Result
We continue to be safe.
Motivation:
Swift 5.9 is no longer supported, we should bump the tools version and
remove it from our CI.
Modifications:
* Bump the Swift tools version to Swift 5.10
* Remove Swift 5.9 jobs where appropriate in main.yml, pull_request.yml
Result:
Code reflects our support window.
Add and enable Swift 6.1 workflows
### Motivation:
Swift 6.1 has been released, we should add it to our CI coverage.
### Modifications:
* Update `appe/swift-nio/scripts/generate_matrix.sh`
* Update reusable adopters of `swift_test_matrix.yml`
* Update end-user adopter workflows of `swift_test_matrix.yml`
* Copy over test flags from Swift 6.0 jobs
### Result:
NIO tests against Swift 6.1 in CI and downstream repositories can
opt-in.
(Successful CI run with the workflows modified to use the changes on
this branch
https://github.com/apple/swift-nio/actions/runs/14400598096?pr=3196)
This PR adds two benchmarks:
- Jumping 1k times from the global executor to a NIO EL and back using
`el.execute {}` and an `UncheckedContinuation`
- Jumping 1k times from the global executor to a NIO EL and back using
an actor that has a custom executor
Co-authored-by: Cory Benfield <lukasa@apple.com>
### Motivation:
Nightly main benchmarks have improved allocations, we should lock in the
win.
### Modifications:
Update nightly main benchmark thresholds
### Result:
Thresholds reflect gains, passing CI.
Following on from https://github.com/apple/swift-nio/pull/3126 delete
`Benchmarks/Thresholds/nightly-6.1` and
`IntegrationTests/tests_04_performance/Thresholds/nightly-6.1.json`
which is no longer needed now that the shared benchmarks workflow has
been updated.
Use nightly_next as swift version
see https://github.com/apple/swift-nio/pull/3122
Motivation:
To not have to rename threshold directories when the nightly branch
changes.
Modifications:
* Use nightly_next as swift version in the matrix generation script
which
is picked up by the benchmark script.
* Move nightly-next thresholds and add legacy symlink
Result:
Benchmark thresholds will attempt to find directories named
nightly_next, not nightly_6_1.
### Motivation:
The HappyEyeballsResolver, being an old part of our stack, has a lot of
code in it that fails to pass strict concurrency checking. That's deeply
suboptimal.
### Modifications:
- Clean up the happy eyeballs resolver under strict concurrency
- Further cleanups to the bootstraps
### Result:
Another step taken on the road to strict concurrency.
### Motivation:
Some changes were missed in #3076, passing through parameters for the
new 6.1 version.
### Modifications:
Pass through more parameters, clean up misleading comments.
### Result:
6.1 nightly runs will be more like 6.0 nightly runs were
### Motivation:
`testClientBindWorksOnSocketsBoundToEitherIPv4OrIPv6Only` would fail
sometimes leaking the IPv4 promise in `GetaddrinfoResolver`
`HappyEyeballsConnector` returns the connection when it resolves either
IPv4 of IPv6. It uses the `GetaddrinfoResolver` which holds a promise
for each of the IPv4 and IPv6 resolution; when one is completed the
connection will be returned and it is possible to start tearing down the
test and shutting down the event loop before the other is completed and
we leak the promise.
### Modifications:
Complete both futures on the event loop rather than the dispatch queue.
### Result:
The futures are completed in the same event loop tick meaning that we
cannot continue execution and leak one.
# 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:
In the past we introduced a memory leak around the creation of and
waiting on futures - we should protect against leaks in this fundamental
operation.
### Modifications:
Add a new benchmark which would have failed with the previous bug
### Result:
Regression protection for this type of memory leak.
## 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 │
╘═══════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛
```
# Motivation
We want to have an aligned strategy across all of our benchmarks in how
we use the scaling factor and samples.
# Modification
This PR makes sure that any inner loop is scaled by the scaling factor.
Moreover, we want to run more than 1 iteration. To do this we set an
almost infinite maximum duration and a maximum iteration count.
# Result
More consistent benchmarks
* 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:
NIOLockedValueBox has a 'safer' API than NIOLock as it only provides
scoped access to its boxed value. NIOLock requires users to only access
protected state while the lock is acquired. As such NIOLockedValueBox
should be preferred where possible. However, there are cases where
manual control must be used (such as storing a continuation) and users
must use a NIOLock for this.
There are two downsides to this:
1. All other access to the protected state must use the NIOLock API
putting the onus on the developer to only access the protected state
while the lock is held.
2. NIOLock can't store its protected state inline which typically
results in users storing it on a class.
Modifications:
- Add an 'unsafe' view to NIOLockedValueBox which allows users to
manually control the lock and access its protected state
- Update NIOAsyncWriter and NIOThrowingAsyncSequenceProducer to use NIOLockedValueBox
Result:
- Safer locking API is used in more places
- Fewer allocations
# Motivation
Another reusable check is to make sure that all library products of a package are successfully building when consumed from a module that has Cxx interoperability enabled. Another check that's missing is running the integration tests.
# Modification
This PR adds two new checks to the reusable workflow. One to check for Cxx interoperability compatibility and another one to run the integration tests. I also fixed a misalgined name for the nightly benchmarks.
# Result
This should be one of the last reusable workflow checks.
* [GHA] Add license header check
# Motivation
We need to make sure all code files have the appropriate license headers in place. This is currently part of the soundness check and we need to replace this with a GH action.
# Modification
Adds a new job to the reusable workflow the check all files for license headers. Since some files are ignored every repo can specify a `.licenseignore` file.
# Result
Last part of the soundness script migrated.
* Review
* Remove default excludes
Motivation:
The NIOAsyncChannel allocates 12 times on init. 4 of these allocations
come from creating two channel handlers and two channel handler
contexts. There's no inherent reason that these channel handlers can't
be combined to eliminate two allocations (one handler and one context).
Modifications:
- Combine `NIOAsyncChannelInboundStreamChannelHandler` and
`NIOAsyncChannelOutboundWriterHandler` into a single
`NIOAsyncChannelHandler`. Most of this was straightforward as only a
few handler operations were duplicated across both.
- Add a 'NIOAsyncChannelHandlerWriterDelegate' in place of the
'NIOAsyncChannelOutboundWriterHandler.Delegate'. One knock on from
this is that the new delegate stores callbacks rather than the
concrete type of the handler. This is necessary to prevent the
generics from the new channel handler bubbling up to the outbound
writer (which would break API and be somewhat odd).
Result:
Fewer allocations
Motivation:
NIOAsyncChannel has a number of allocs associated with it. We should
have a benchmark which tracks the number of allocs.
Modifications:
- Add NIOCoreBenchmarks with a single benchmark
Result:
Better insight
# Motivation
We had to disable the benchmarks since they regressed without us noticing and they appear to be flaky.
# Modification
This PR fixes the allocation regression and tries to re-enable them.
* Set `SWIFT_VERSION` environment variable to resolve to the correct benchmarks thresholds path
* mallocs have increased
* update benchmark results manually
* update thresholds again
* disable flaky benchmark
* Add `closeOnDeinit` to the `NIOAsyncChannel` init
# Motivation
In my previous PR, I already did the work to add `finishOnDeinit` configuration to the `NIOAsyncWriter` and `NIOAsyncSequenceProducer`. This PR also automatically migrated the `NIOAsyncChanell` to set the `finishOnDeinit = false`. This was intentional since we really want users to not use the deinit based cleanup; however, it also broke all current adopters of this API semantically and they might now run into the preconditions.
# Modification
This PR reverts the change in `NIOAsyncChannel` and does the usual deprecate + new init dance to provide users to configure this behaviour while still nudging them to check that this is really what they want.
# Result
Easier migration without semantically breaking current adopters of `NIOAsyncChannel`.
* Rename to `wrappingChannelSynchronously`
* Add `withInboundOutboud` to `NIOAsyncChannel` and deprecate deinit based cleanup
# Motivation
We just released our new async NIO APIs and have already gotten quite a bunch of feedback from adopters. One of the feedback was that the deinit based closing that we have added to the `NIOAsyncChannel` has caused problems since it leads to unexpected closure of their `Channel`. Furthermore, it makes it impossible to determine how many open sockets a program has at any given time since deinit based clean up relies on the optimizer and can happen at random times.
# Modifications
This PR adds new inits to `NIOAsyncSequenceProducer` and `NIOAsyncWriter` which disable the `deinit` based clean up and instead replace them with an assertion. This allows developers to still catch these issues at debug time. Furthermore, I added a new `withInboundOutbound` scoped access to `NIOAsyncChannel` which will close the channel at the end of the scope. This still gives users a nice API while not having to care much about closing themselves.
# Result
We are no longer using deinit based clean up and bring back one of the core principles of NIO which is deterministic resource usage.
* Review comments
* Internal labels for closure arguments
* Rename to `executeThenCloseChannel`
* Actually call `sinkDeinitialized`
* Change preconditions
* Move logic to deinits
* Rename to `executeThenClose` and review nits
# Motivation
Over the past months, we have been working on new async bridges to make using NIO's `Channel` from Swift Concurrency possible. Since this work was far reaching we have opted to land all of it as SPI. Now the time has come and we feel confident enough to make the SPI official API. This comes after testing the new APIs in various scenarios such as HTTP 1&2, HTTP upgrades, protocol negotiation and in benchmarks.
# Modification
This PR removes the SPI from the `NIOAsyncChannel`, the bootstrap methods, protocol negotiation and HTTP upgrade.
# Result
Everyone can use the our new APIs🚀
* Call `NIOAsyncWriterSinkDelegate` outside of the lock
# Motivation
The current `NIOAsyncWriter` implementation expects that the delegate is called while holding the lock to avoid reentrancy issues. However, this prevents us from executing the delegate calls directly on the `EventLoop` if we are on it already.
# Modification
This moves all of the delegate calls outside of the locks and adds protection against reentrancy into the state machine.
# Result
Less allocations.
Clarify the reentrancy problems in docs and protect against them in the writer
* Code review
* Implement fast paths for `AsyncChannelInboundStreamChannelHandler`
* Add `_TinyArray` from `swift-certificates`
* Implement single element customization point in `NIOAsyncChannelOutboundWriterHandler`
* Call the single element optimization more often and store suspended producers in `_TinyArray`.
* Update thresholds
* Fix compiler warning
# 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`.
# Motivation
We want to benchmark our `NIOAsyncChannel` to see how it compares to the synchronous implementation with `ChannelHandler`s
# Modification
This PR adds a new `TCPEchoAsyncChannel` benchmark that mimics the `TCPEcho` benchmark but uses our new async bridges.
Since Swift Concurrency, is normally using a global executor this benchmark would have quite high variation. To reduce this variant I introduced code to hook the global executor and set an `EventLoop` as the executor. In the future, if we get task executors we can change the code to us them instead.
# Result
New baseline benchmarks for the `NIOAsyncChannel`.
# Motivation
We want to migrate our allocation and later on also our performance tests to use the `package-benchmark` plugin. This plugin makes writing benchmarks way easier than our current setup. Furthermore, debugging benchmarks is also possible from within Xcode now.
# Modification
This PR adds the setup for the benchmarking infrastructure and connects it with out
# Result
Allocations tests are more accessible and easier to iterate.
Fabian Fett
Kushal Pisavadia
Cory BenfieldandGeorge Barnett
Rick Newton-Rogers
Franz Busch
Si Beaumont
Johannes Weiss
David Nadoba