### Motivation:
In issue https://github.com/apple/swift-nio/issues/1316, we see a large number of allocations to happen when scheduling tasks. This can definitely be optimized. This PR adds a number of baseline allocation and performance tests for both `scheduleTask` and `execute`. In the next PRs, I am going to try a few optimizations to reduce the number of allocations.
### Modifications:
Added baseline performance and allocation tests for `scheduleTask` and `execute`
Motivation:
Frequently when folks have issues with SwiftNIO or related technologies,
we need to ask the same questions over and over again.
It'd be much nicer to just have a script that collects them.
Modifications:
Add `scripts/nio-diagnose`.
Result:
Users/devs can easily create nio-diagnose files which are markdown and
can easily be pasted into GH.
Motivation:
Newer copies of liburing have changed header imports, making some
of our implicit includes fail, and have also changed the types
of some methods in a way that would only produce warnings in C but
which produces errors in Swift. Additionally, liburing broke when
we did the module split, but we didn't notice.
Modifications:
- Import NIOCore for the Uring Swift code.
- Import poll.h in CNIOLinux
- Support arguments that are either pointers or UInt64s.
Results:
liburing code compiles again
* Change Swift Concurrency Availability
* Label GitHub Action Jobs
* Incorporate the insights from Johannes Weiss
* Remove GitHub Actions
* Incorporate the feedback of Cory Benfield
Co-authored-by: Cory Benfield <lukasa@apple.com>
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:
It's like that #1999 was a little wide, and we should have forbidden
EBDAF on close.
Modifications:
Still forbid EBADF on close.
Result:
EBADF still preconditions on close.
Motivation:
`pthread_mutex_t` locks can do advanced error checking with
`PTHREAD_MUTEX_ERRORCHECK`, the cost of which was unknown. With #1994 I
could now measure it.
The differences from my local machine (M1 MBP) for uncontended locks
are:
- on Linux: 10% faster without `PTHREAD_MUTEX_ERRORCHECK`
- on macOS: 25% faster without `PTHREAD_MUTEX_ERRORCHECK`!
Modification:
Do the extended error checking only in debug mode. Note that we still
`precondition` on all pthread return values (which is basically free).
Result:
Faster code.
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
To judge the cost of `PTHREAD_MUTEX_ERRORCHECK` as well as
`os_unfair_lock` vs `pthread_mutex_t` it's useful to have a few simple
benchmarks.
Modification:
Add lock benchmarks.
Result:
Better data.
Motivation:
This test is flaky, and has always been flaky.
The issue here is simply a timing one. It has always been possible
for the thread running in the background DispatchQueue to be take
longer between calling semaphore.signal and promise.cascade(to:)
than it does for the event loop to process the tasks. If that
happens, this test will fail by eventually timing out.
Modifications:
- Adjust the test to avoid using promise.cascade(to:), and so prevent it
from having at timing window.
Result:
The test will be not flaky, or at least less flaky.
Resolves#1971.
Co-authored-by: George Barnett <gbarnett@apple.com>
Motivation:
ByteBuffer is quite core to NIO programs' perf and yet it doesn't even
try to reduce the number of under/overflow & bounds checks.
That's a lot of code that will never be run and that also introduces
unncessary branches which hurt perf.
Modification:
Remove a bunch of low hanging fruit checks.
Result:
Faster (and less) code.
Motivation:
Many network protocols (especially for example NFS) have quite a number
of integer values next to each other. In NIO, you'd normally parse/write
them with multiple read/writeInteger calls.
Unfortunately, that's a bit wasteful because we're checking the bounds
as well as the CoW state every time.
Modifications:
- Provide read/writeMultipleIntegers for up to 15 FixedWidthIntegers.
- Benchmarks
Result:
Faster code. For 10 UInt32s, this is a 5x performance win on my machine,
see benchmarks.
Motivation:
- Currently http 1xx response heads are handled incorrectly
Modifications:
- Add an `InformationalResponseStrategy` that allows the user to specify whether http 1xx responses shall be forwarded or dropped.
- Implement the necessary cases in `HTTPDecoder` and `BetterHTTPParser`
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
I noticed that when replacing `_UInt24` to `UInt32` in `ByteBuffer`'s
code (which unfortunately increases its size and is therefore
unacceptable), then `getSlice` gets fully inlined into `readSlice`.
But in the normal `ByteBuffer` code (with `_UInt24`) that is not the
case. So I had a quick go a shrinking the `getSlice` code. Sadly, it's
still not quite enough to get `getSlice` inlined into `readSlice`.
There are quite a lot of extra shifts for the UInt32 <--> UInt24
conversions and an extra branch (to check the `sliceBeginIndex >= 16
MiB` condition).
Modifications:
- use unchecked arithmetic where safe
- outline the slow path
Result:
much smaller code size
In my local testing I have seen that the compiler adds a retain and release cycle around the use of `Optional.map` in ByteBuffer's read methods.
Modifications:
- Replaced the use of `Optional.map` with `guard let` in ByteBuffer's read methods.
Motivation:
Currently you cannot create a NIOPipeBootstrap when you are on
an event loop due to a precondition check. This check seeks to prevent
consumers of the API from passing in a file descriptor thats referencing
a file on disk or on the network.
The method at hand 'validateFileDescriptorIsNotAFile' uses fstat, which potentially
could block the event loop especially if the fd is referencing a file over the network.
This check however prevents certain use cases of SwiftNIO, where the downsides
of a consumer of this api blocking their own event loop do not weigh in against preventing
an entire use case from SwiftNIO.
Modifications:
Removed the precondition
Result:
After this change it will be possible to feed file descriptors into
the bootstrap which potentially block the current event loop.
A potential (portable) replacement for fstat still has to be found in order to solve this problem completely.
Motivation:
Swift 5.5 provides concurrency support on almost all platforms where it
is available. However, the Xcode 13 GM currently provides a macOS 11 SDK
with Swift 5.5, and the concurrency features are not available in that
SDK. As a result, NIO's concurrency features cause compile errors when
building the concurrency library on macOS using Xcode 13 GM.
Modifications:
- Only build the concurrency features when the concurrency library is
present.
Result:
We can build NIO using Xcode 13 GM.
Motivation:
In February, @weissi noticed that the nightly builds of Swift could no
longer compile the unit tests. We disabled them at that time and
reported an upstream bug, https://bugs.swift.org/browse/SR-14268. This
is not an ideal situation, but the Swift core team has provided a
workaround we can use to re-enable the tests.
Modifications:
Provide free functions that wrap some specific extension methods that
are called in unit tests, and amend the call sites to use them.
Result:
We can re-enable the nightly tests.
Same issue as fixed#1733 for ByteToMessageHandler.
### Motivation:
`NIOSingleStepByteToMessageProcessor` called out to the decoder's shouldReclaimBytes method after every parsing attempt, even if the decoder returned a value (which means continue in the `decodeLoop`).
That's quite pointless because we won't add any bytes into the buffer before we're trying the decoder again. Further it is a huge performance penalty for a use-cases in which we only consume small frames from the buffer. (Like reading data rows in a database client)
### Modifications:
- Only ask the decoder if we should reclaim bytes if the decoder actually is not able to process more frames from the input buffer.
Motivation:
Xcode 13 GM shipped with a Swift overlay for libsystem in macOS that
marked free's first argument as non-nullable. This leads to an awkward
breakage for us, because we're trying to hold a reference to free as a
function pointer, and to do that we had an explicit type annotation.
We'd like to keep NIO compiling in Xcode 13 GM.
Modifications:
- Defined the free function as a thunk that passes through to the
underlying OS free call, but takes its first argument as non-nullable.
Result:
Should compile on the Xcode 13 GM again.
Motivation:
Follow up to https://github.com/apple/swift-nio/pull/1952#discussion_r707351015
The OWS rule from the HTTP semantics draft only considers 'SP' and
'HTAB' to be whitespace. We also (unnecessarily) consider CR and LF to
be whitespace.
Modifications:
- Remove CR and LF from the characters we consider to be whitespace
- Update tests
Result:
We no longer consider CR or LF to be whitespace when trimming
whitespace when producing the canonical form of header values.
Motivation:
When getting the canonical form of header values any ascii whitespace
is stripped from the values. The current implementation does this by
doing equality checks on `Character` which is quite expensive.
Modifications:
- Update the `Substring.trimWhitespace()` function to trim on a UTF8
view
Result:
Retrieving the canonical form of header values is cheaper, significantly
so when values contain whitespace.
Motivation:
To justify performance changes we need to measure the code being
changed. We believe that `HTTPHeaders.subscript(canonicalForm:)` is a
little slow.
Modifications:
- Add allocation and performance tests for fetching header values in
their canonical form
Results:
More benchmarks!
Motivation:
It's currently unclear that SwiftNIO1 is end of life.
Modifications:
Update the README to set a deadline on patches to NIO1
Result:
Everyone knows where the core team stands on supporting NIO1
Co-authored-by: George Barnett <gbarnett@apple.com>
Motivation:
In #1888 a C compiler setting was added to Package.swift for CNIODarwin.
This isn't reflected in the associated CNIODarwin podspec so the pod
fails to build.
Modifications:
Modify the build_podspecs.sh script to add the missing compiler setting
for CNIODarwin.
Result:
CNIODarwin pod builds.
Motivation:
Our kqueue filter set differ logic attempted to stack allocate some
kevent objects and then treat them as a fixed size array by creating a
tuple and then grabbing a pointer to it. That's fairly gratuitously
unsafe, and we should shrink the amount of code that can touch unsafe
stuff.
Modifications:
- Defined KeventTriple, a value type that encapsulates a fixed-size
array and a length.
- Rewrote calculateKQueueFilterSetChanges to use the new type.
Result:
Safer code in the NIO core
Co-authored-by: George Barnett <gbarnett@apple.com>
* Fix doc generation and jazzy version
From: https://github.com/apple/swift-nio-extras/pull/141
motivation: fix ci
changes:
* only install ruby and jazzy on focal since jazzy id not supported onlder versions of ubuntu
* fix doc generations script adjusting it to latest source-kittent syntax
* Always install ruby: we need it for generating a test manifest
- Use correct @available checks in `NIOAsyncAwaitDemo` (post WWDC '21)
- Remove `import _Concurrency` statements
Result:
- Cleaner imports
- static-stdlib linking works again on 5.5 (it does not if we explicitly `import _Concurrency`)
Co-authored-by: George Barnett <gbarnett@apple.com>
Motivation:
We should explain when we want users to use specific products, so that
they can understand what to import and when.
Modifications:
- Added some text to the README to clarify what modules exist.
Result:
Should be easier to understand what there is in the NIO ecosystem
Co-authored-by: Fabian Fett <fabianfett@apple.com>
Motivation:
The remaining NIO code really conceptually belongs in a module called
NIOPosix, and NIOCore should really be called NIO. We can't really do
that last step, but we can prepare by pushing the bulk of the remaining
code into a module called NIOPosix.
Modifications:
- Move NIO to NIOPosix
- Make NIO an umbrella module.
Result:
NIOPosix exists.
Motivation:
In #1935 I removed NIO as a dependency of a number of our library
targets. This unfortunately led to some downstream breakage in modules
that were (incorrectly) assuming they could import NIO without
expressing a dependency on it in their Package.swift, e.g.
swift-server/swift-aws-lambda-runtime#218.
While a forums thread
(https://forums.swift.org/t/semantic-versioning-should-removing-a-dependency-be-a-semver-major/51179)
is ongoing to discuss the implications of this, we should re-add the
dependency to undo the breakage against main. I've validated this
locally: merely having the dependency is enough, we don't have to use
it.
Modifications:
- Re-add NIO as a dependency to:
- _NIOConcurrency
- NIOFoundationCompat
- NIOHTTP1
- NIOTLS
- NIOWebSocket
Result:
Broken downstreams can build again.
Motivation:
As we've largely completed our move to split out our core abstractions,
we now have an opportunity to clean up our dependencies and imports. We
should arrange for everything to only import NIO if it actually needs
it, and to correctly express dependencies on NIOCore and NIOEmbedded
where they exist.
We aren't yet splitting out tests that only test functionality in
NIOCore, that will follow in a separate patch.
Modifications:
- Fixed up imports
- Made sure our protocols only require NIOCore.
Result:
Better expression of dependencies.
Co-authored-by: George Barnett <gbarnett@apple.com>
Motivation:
EmbeddedChannel is an important testing tool, and we want to use it
without needing to bring along the POSIX layer. They are not tightly
coupled. However, it also doesn't belong naturally in NIOCore, so we
should probably put it in its own place.
Modifications:
- Moved EmbeddedChannel and EmbeddedEventLoop to NIOEmbedded.
- Moved the tests to NIOEmbeddedTests
- Duplicated some test helpers
Result:
Easy to use EmbeddedChannel without the POSIX layer.
Motivation:
I'd like to move EmbeddedChannel and friends out of the main NIO
repository and into their own module. Unfortunately, EmbeddedChannel
shares the PriorityQueue implementation we wrote with the various POSIX
channels. To avoid duplicating the code, we should pull it out to its
own module.
However, we've never wanted to commit to the API of this data structure,
and the same is true now. To that end, I'm pulling it into an
underscored module that is not a product of the package.
We could have used the `@_spi` annotation here but honestly I'm a bit
nervous about doing that at the low-level of NIO itself, as if the Swift
team does change the spelling of it at any point in the future we'll be
in real trouble. This way works almost as well, and makes our intent a
lot clearer.
Modifications:
- Extracted Heap and PriorityQueue to a new module.
- Made everything @inlinable to do our best to make performance
acceptable.
Result:
We can re-use PriorityQueue.
Franz Busch