Create platform specific AsyncIO #64
Conversation
| #if os(Windows) | ||
| // Compiler generated conformances | ||
| #else | ||
| #if canImport(Darwin) |
There was a problem hiding this comment.
I know these aren't not new changes, but why do we have custom implementation only for Darwin?
There was a problem hiding this comment.
DispatchData is unfortunately not Hashable
| ) async throws -> [UInt8]? { | ||
| // If we are reading until EOF, start with readBufferSize | ||
| // and gradually increase buffer size | ||
| let bufferLength = maxLength == .max ? readBufferSize : maxLength |
There was a problem hiding this comment.
Can we just respect the passed-in maxLength as-is and not impose this silent cap?
This feels deceiving from callsites perspective and I imagine it'd make it difficult to figure out what's going on
There was a problem hiding this comment.
Here we need to preallocate buffer of a certain length. We can't just pass in .max to allocate infinite buffer so we have to grow it manually similar like an array (all this is so we can avoid copying and write into the buffer directly from read()).
| ) | ||
| } | ||
|
|
||
| self.setupError = maybeSetupError |
There was a problem hiding this comment.
Why can't we throw this error right here in init?
There was a problem hiding this comment.
AsyncIO is designed to be a singleton static let so we only spin up one additional thread for monitoring. Unfortunately this also means we can't have failable initializer...
|
What's the strategy for other Unix platforms like FreeBSD? |
| [PlatformFileDescriptor : SignalStream.Continuation] | ||
| > = Mutex([:]) | ||
|
|
||
| final class AsyncIO: Sendable { |
There was a problem hiding this comment.
On Darwin, class AsyncIO doesn't have any stored variables. That means AsyncIO.shared.read() can also be a static function, and you would just write AsyncIO.read(...)
I think you left it this way because on linux it has stored variables so they can't be static funcs, but we still want to share the same source at callsites? If so would it make more sense to move the #if platform guards into the shared functions? So instead of
#if linux
class AsyncIO {
func read(...)
}
#elif mac
class AsyncIO {
func read(...)
}
#endif
Can we have
class AsyncIO {
func read(...) {
#if linux
...
#elif mac
...
#endif
}
}
There was a problem hiding this comment.
I decided to go with the first approach because the implementation for each platform is vastly different. So having #if #else inside of each function makes this file overall not readable. This is especially true now I'm adding Windows-specific implementation on top of this. It's much more maintainable if each platform has its own "section" of code so the overall flow is more clear.
We don't currently have a plan to support |
- Darwin: based on DispatchIO - Linux: based on epoll - Windows (not included in this commit): based on IOCP with OVERLAPPED
iCharlesHu
force-pushed
the
charles/linux-async-io
branch
from
893f4ed to
17afb54
Compare
|
Rebased |
This PR introduces platform specific
AsyncIO:The
epollbasedAsyncIOreplacesDispatchIOon Linux.Resolves: #47