Skip to content

Commit

Permalink
require ChannelOptions to be Equatable (#823)
Browse files Browse the repository at this point in the history 
Motivation:

ChannelOptions should've always been Equatable and so far we've hacked
around them not being Equatable when we wanted to compare them.

Modifications:

make all ChannelOptions Equtable

Result:

- ChannelOption comparison actually works
- fixes #598
- make more ChannelOptions stuff inlinable to not regress allocations
  • Loading branch information
@weissi
weissi authored Feb 19, 2019
1 parent 03aa0bd commit 877f0eb
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 70 deletions.
76 changes: 34 additions & 42 deletions Sources/NIO/Bootstrap.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@ public final class ServerBootstrap {
private let childGroup: EventLoopGroup
private var serverChannelInit: ((Channel) -> EventLoopFuture<Void>)?
private var childChannelInit: ((Channel) -> EventLoopFuture<Void>)?
private var serverChannelOptions = ChannelOptionStorage()
private var childChannelOptions = ChannelOptionStorage()
@usableFromInline
internal var _serverChannelOptions = ChannelOptionStorage()
@usableFromInline
internal var _childChannelOptions = ChannelOptionStorage()

/// Create a `ServerBootstrap` for the `EventLoopGroup` `group`.
///
Expand Down Expand Up @@ -119,8 +121,9 @@ public final class ServerBootstrap {
/// - parameters:
/// - option: The option to be applied.
/// - value: The value for the option.
public func serverChannelOption<T: ChannelOption>(_ option: T, value: T.OptionType) -> Self {
serverChannelOptions.put(key: option, value: value)
@inlinable
public func serverChannelOption<Option: ChannelOption>(_ option: Option, value: Option.OptionType) -> Self {
self._serverChannelOptions.put(key: option, value: value)
return self
}

Expand All @@ -129,8 +132,9 @@ public final class ServerBootstrap {
/// - parameters:
/// - option: The option to be applied.
/// - value: The value for the option.
public func childChannelOption<T: ChannelOption>(_ option: T, value: T.OptionType) -> Self {
childChannelOptions.put(key: option, value: value)
@inlinable
public func childChannelOption<Option: ChannelOption>(_ option: Option, value: Option.OptionType) -> Self {
self._childChannelOptions.put(key: option, value: value)
return self
}

Expand Down Expand Up @@ -201,10 +205,10 @@ public final class ServerBootstrap {
private func bind0(makeServerChannel: (_ eventLoop: SelectableEventLoop, _ childGroup: EventLoopGroup) throws -> ServerSocketChannel, _ register: @escaping (EventLoop, ServerSocketChannel) -> EventLoopFuture<Void>) -> EventLoopFuture<Channel> {
let eventLoop = self.group.next()
let childEventLoopGroup = self.childGroup
let serverChannelOptions = self.serverChannelOptions
let serverChannelOptions = self._serverChannelOptions
let serverChannelInit = self.serverChannelInit ?? { _ in eventLoop.makeSucceededFuture(()) }
let childChannelInit = self.childChannelInit
let childChannelOptions = self.childChannelOptions
let childChannelOptions = self._childChannelOptions

let serverChannel: ServerSocketChannel
do {
Expand Down Expand Up @@ -346,7 +350,8 @@ public final class ClientBootstrap {

private let group: EventLoopGroup
private var channelInitializer: ((Channel) -> EventLoopFuture<Void>)?
private var channelOptions = ChannelOptionStorage()
@usableFromInline
internal var _channelOptions = ChannelOptionStorage()
private var connectTimeout: TimeAmount = TimeAmount.seconds(10)
private var resolver: Resolver?

Expand Down Expand Up @@ -385,8 +390,9 @@ public final class ClientBootstrap {
/// - parameters:
/// - option: The option to be applied.
/// - value: The value for the option.
public func channelOption<T: ChannelOption>(_ option: T, value: T.OptionType) -> Self {
channelOptions.put(key: option, value: value)
@inlinable
public func channelOption<Option: ChannelOption>(_ option: Option, value: Option.OptionType) -> Self {
self._channelOptions.put(key: option, value: value)
return self
}

Expand Down Expand Up @@ -477,7 +483,7 @@ public final class ClientBootstrap {
}

return channelInitializer(channel).flatMap {
self.channelOptions.applyAll(channel: channel)
self._channelOptions.applyAll(channel: channel)
}.flatMap {
let promise = eventLoop.makePromise(of: Void.self)
channel.registerAlreadyConfigured0(promise: promise)
Expand All @@ -494,7 +500,7 @@ public final class ClientBootstrap {
protocolFamily: Int32,
_ body: @escaping (Channel) -> EventLoopFuture<Void>) -> EventLoopFuture<Channel> {
let channelInitializer = self.channelInitializer ?? { _ in eventLoop.makeSucceededFuture(()) }
let channelOptions = self.channelOptions
let channelOptions = self._channelOptions

let promise = eventLoop.makePromise(of: Channel.self)
let channel: SocketChannel
Expand Down Expand Up @@ -556,7 +562,8 @@ public final class DatagramBootstrap {

private let group: EventLoopGroup
private var channelInitializer: ((Channel) -> EventLoopFuture<Void>)?
private var channelOptions = ChannelOptionStorage()
@usableFromInline
internal var _channelOptions = ChannelOptionStorage()

/// Create a `DatagramBootstrap` on the `EventLoopGroup` `group`.
///
Expand All @@ -581,8 +588,9 @@ public final class DatagramBootstrap {
/// - parameters:
/// - option: The option to be applied.
/// - value: The value for the option.
public func channelOption<T: ChannelOption>(_ option: T, value: T.OptionType) -> Self {
channelOptions.put(key: option, value: value)
@inlinable
public func channelOption<Option: ChannelOption>(_ option: Option, value: Option.OptionType) -> Self {
self._channelOptions.put(key: option, value: value)
return self
}

Expand Down Expand Up @@ -651,7 +659,7 @@ public final class DatagramBootstrap {
private func bind0(makeChannel: (_ eventLoop: SelectableEventLoop) throws -> DatagramChannel, _ registerAndBind: @escaping (EventLoop, DatagramChannel) -> EventLoopFuture<Void>) -> EventLoopFuture<Channel> {
let eventLoop = self.group.next()
let channelInitializer = self.channelInitializer ?? { _ in eventLoop.makeSucceededFuture(()) }
let channelOptions = self.channelOptions
let channelOptions = self._channelOptions

let channel: DatagramChannel
do {
Expand All @@ -672,53 +680,37 @@ public final class DatagramBootstrap {
}
}

@usableFromInline
/* for tests */ internal struct ChannelOptionStorage {
private var storage: [(Any, (Any, (Channel) -> (Any, Any) -> EventLoopFuture<Void>))] = []

mutating func put<K: ChannelOption & Equatable>(key: K, value: K.OptionType) {
return self.put(key: key, value: value, equalsFunc: ==)
}

// HACK: this function should go for NIO 2.0, all ChannelOptions should be equatable
mutating func put<K: ChannelOption>(key: K, value: K.OptionType) {
if K.self == SocketOption.self {
return self.put(key: key as! SocketOption, value: value as! SocketOptionValue) { lhs, rhs in
switch (lhs, rhs) {
case (.const(let lLevel, let lName), .const(let rLevel, let rName)):
return lLevel == rLevel && lName == rName
}
}
} else {
return self.put(key: key, value: value) { _, _ in true }
}
}
@usableFromInline
internal var _storage: [(Any, (Any, (Channel) -> (Any, Any) -> EventLoopFuture<Void>))] = []

@inlinable
mutating func put<K: ChannelOption>(key: K,
value newValue: K.OptionType,
equalsFunc: (K, K) -> Bool) {
value newValue: K.OptionType) {
func applier(_ t: Channel) -> (Any, Any) -> EventLoopFuture<Void> {
return { (x, y) in
return t.setOption(option: x as! K, value: y as! K.OptionType)
}
}
var hasSet = false
self.storage = self.storage.map { typeAndValue in
self._storage = self._storage.map { typeAndValue in
let (type, value) = typeAndValue
if type is K && equalsFunc(type as! K, key) {
if type is K && type as! K == key {
hasSet = true
return (key, (newValue, applier))
} else {
return (type, value)
}
}
if !hasSet {
self.storage.append((key, (newValue, applier)))
self._storage.append((key, (newValue, applier)))
}
}

func applyAll(channel: Channel) -> EventLoopFuture<Void> {
let applyPromise = channel.eventLoop.makePromise(of: Void.self)
var it = self.storage.makeIterator()
var it = self._storage.makeIterator()

func applyNext() {
guard let (key, (value, applier)) = it.next() else {
Expand Down
45 changes: 26 additions & 19 deletions Sources/NIO/ChannelOption.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
//===----------------------------------------------------------------------===//

/// A configuration option that can be set on a `Channel` to configure different behaviour.
public protocol ChannelOption {
public protocol ChannelOption: Equatable {
associatedtype AssociatedValueType
associatedtype OptionType

Expand Down Expand Up @@ -70,22 +70,29 @@ public enum SocketOption: ChannelOption {
return (level, name)
}
}

public static func == (lhs: SocketOption, rhs: SocketOption) -> Bool {
switch (lhs, rhs) {
case (.const(let lLevel, let lName), .const(let rLevel, let rName)):
return lLevel == rLevel && lName == rName
}
}
}

/// `AllocatorOption` allows to specify the `ByteBufferAllocator` to use.
public enum AllocatorOption: ChannelOption {
public typealias AssociatedValueType = ()
public typealias OptionType = ByteBufferAllocator

case const(())
case const
}

/// `RecvAllocatorOption` allows users to specify the `RecvByteBufferAllocator` to use.
public enum RecvAllocatorOption: ChannelOption {
public typealias AssociatedValueType = ()
public typealias OptionType = RecvByteBufferAllocator

case const(())
case const
}

/// `AutoReadOption` allows users to configure if a `Channel` should automatically call `Channel.read` again once all data was read from the transport or
Expand All @@ -94,7 +101,7 @@ public enum AutoReadOption: ChannelOption {
public typealias AssociatedValueType = ()
public typealias OptionType = Bool

case const(())
case const
}

/// `WriteSpinOption` allows users to configure the number of repetitions of a only partially successful write call before considering the `Channel` not writable.
Expand All @@ -104,7 +111,7 @@ public enum WriteSpinOption: ChannelOption {
public typealias AssociatedValueType = ()
public typealias OptionType = UInt

case const(())
case const
}

/// `MaxMessagesPerReadOption` allows users to configure the maximum number of read calls to the underlying transport are performed before wait again until
Expand All @@ -113,15 +120,15 @@ public enum MaxMessagesPerReadOption: ChannelOption {
public typealias AssociatedValueType = ()
public typealias OptionType = UInt

case const(())
case const
}

/// `BacklogOption` allows users to configure the `backlog` value as specified in `man 2 listen`. This is only useful for `ServerSocketChannel`s.
public enum BacklogOption: ChannelOption {
public typealias AssociatedValueType = ()
public typealias OptionType = Int32

case const(())
case const
}

/// The watermark used to detect when `Channel.isWritable` returns `true` or `false`.
Expand Down Expand Up @@ -162,7 +169,7 @@ public enum WriteBufferWaterMarkOption: ChannelOption {
public typealias AssociatedValueType = ()
public typealias OptionType = WriteBufferWaterMark

case const(())
case const
}

/// `ConnectTimeoutOption` allows users to configure the `TimeAmount` after which a connect will fail if it was not established in the meantime. May be
Expand All @@ -171,7 +178,7 @@ public enum ConnectTimeoutOption: ChannelOption {
public typealias AssociatedValueType = ()
public typealias OptionType = TimeAmount?

case const(())
case const
}

/// `AllowRemoteHalfClosureOption` allows users to configure whether the `Channel` will close itself when its remote
Expand All @@ -183,7 +190,7 @@ public enum AllowRemoteHalfClosureOption: ChannelOption {
public typealias AssociatedValueType = ()
public typealias OptionType = Bool

case const(())
case const
}

/// Provides `ChannelOption`s to be used with a `Channel`, `Bootstrap` or `ServerBootstrap`.
Expand All @@ -192,29 +199,29 @@ public struct ChannelOptions {
public static let socket = { (level: SocketOptionLevel, name: SocketOptionName) -> SocketOption in .const((level, name)) }

/// - seealso: `AllocatorOption`.
public static let allocator = AllocatorOption.const(())
public static let allocator = AllocatorOption.const

/// - seealso: `RecvAllocatorOption`.
public static let recvAllocator = RecvAllocatorOption.const(())
public static let recvAllocator = RecvAllocatorOption.const

/// - seealso: `AutoReadOption`.
public static let autoRead = AutoReadOption.const(())
public static let autoRead = AutoReadOption.const

/// - seealso: `MaxMessagesPerReadOption`.
public static let maxMessagesPerRead = MaxMessagesPerReadOption.const(())
public static let maxMessagesPerRead = MaxMessagesPerReadOption.const

/// - seealso: `BacklogOption`.
public static let backlog = BacklogOption.const(())
public static let backlog = BacklogOption.const

/// - seealso: `WriteSpinOption`.
public static let writeSpin = WriteSpinOption.const(())
public static let writeSpin = WriteSpinOption.const

/// - seealso: `WriteBufferWaterMarkOption`.
public static let writeBufferWaterMark = WriteBufferWaterMarkOption.const(())
public static let writeBufferWaterMark = WriteBufferWaterMarkOption.const

/// - seealso: `ConnectTimeoutOption`.
public static let connectTimeout = ConnectTimeoutOption.const(())
public static let connectTimeout = ConnectTimeoutOption.const

/// - seealso: `AllowRemoteHalfClosureOption`.
public static let allowRemoteHalfClosure = AllowRemoteHalfClosureOption.const(())
public static let allowRemoteHalfClosure = AllowRemoteHalfClosureOption.const
}
9 changes: 0 additions & 9 deletions Tests/NIOTests/ChannelTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2726,12 +2726,3 @@ fileprivate class VerifyConnectionFailureHandler: ChannelInboundHandler {
ctx.fireChannelUnregistered()
}
}

extension SocketOption: Equatable {
public static func == (lhs: SocketOption, rhs: SocketOption) -> Bool {
switch (lhs, rhs) {
case (.const(let lLevel, let lName), .const(let rLevel, let rName)):
return lLevel == rLevel && lName == rName
}
}
}
1 change: 1 addition & 0 deletions docs/public-api-changes-NIO1-to-NIO2.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,4 @@
- renamed `EventLoopFuture.hopTo(eventLoop:)` to `EventLoopFuture.hop(to:)`
- `EventLoopFuture.reduce(into:_:eventLoop:_:)` had its label signature changed to `EventLoopFuture.reduce(into:_:on:_:)`
- `EventLoopFuture.reduce(_:_:eventLoop:_:` had its label signature changed to `EventLoopFuture.reduce(_:_:on:_:)`
- all `ChannelOption`s are now required to be `Equatable`

0 comments on commit 877f0eb

Please sign in to comment.