Skip to content

Commit

Permalink
don't capture in the callout closures in BaseSocketChannel (apple#658)
Browse files Browse the repository at this point in the history
Motivation:

We were complaining that we can't use `@convention(thin)` in a place
where we really need to make sure we don't get allocations for returning
a closure (that doesn't capture any context), so it should just be a
function pointer.

In reality however, we were actually capturing context...

Modifications:

Don't capture context anymore.

Result:

Closures should not actually not enclose anything.
  • Loading branch information
weissi authored and Lukasa committed Nov 21, 2018
1 parent cf958fb commit c66386e
Showing 1 changed file with 34 additions and 32 deletions.
66 changes: 34 additions & 32 deletions Sources/NIO/BaseSocketChannel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,61 +68,73 @@ private struct SocketChannelLifecycleManager {
}

@inline(__always) // we need to return a closure here and to not suffer from a potential allocation for that this must be inlined
internal mutating func beginRegistration(promise: EventLoopPromise<Void>?) -> ((ChannelPipeline) -> Void) {
return self.moveState(event: .beginRegistration, promise: promise)
internal mutating func beginRegistration() -> ((EventLoopPromise<Void>?, ChannelPipeline) -> Void) {
return self.moveState(event: .beginRegistration)
}

@inline(__always) // we need to return a closure here and to not suffer from a potential allocation for that this must be inlined
internal mutating func finishRegistration(promise: EventLoopPromise<Void>?) -> ((ChannelPipeline) -> Void) {
return self.moveState(event: .finishRegistration, promise: promise)
internal mutating func finishRegistration() -> ((EventLoopPromise<Void>?, ChannelPipeline) -> Void) {
return self.moveState(event: .finishRegistration)
}

@inline(__always) // we need to return a closure here and to not suffer from a potential allocation for that this must be inlined
internal mutating func close(promise: EventLoopPromise<Void>?) -> ((ChannelPipeline) -> Void) {
return self.moveState(event: .close, promise: promise)
internal mutating func close() -> ((EventLoopPromise<Void>?, ChannelPipeline) -> Void) {
return self.moveState(event: .close)
}

@inline(__always) // we need to return a closure here and to not suffer from a potential allocation for that this must be inlined
internal mutating func activate(promise: EventLoopPromise<Void>?) -> ((ChannelPipeline) -> Void) {
return self.moveState(event: .activate, promise: promise)
internal mutating func activate() -> ((EventLoopPromise<Void>?, ChannelPipeline) -> Void) {
return self.moveState(event: .activate)
}

// MARK: private API
@inline(__always) // we need to return a closure here and to not suffer from a potential allocation for that this must be inlined
private mutating func moveState(event: Event, promise: EventLoopPromise<Void>?) -> ((ChannelPipeline) -> Void) {
private mutating func moveState(event: Event) -> ((EventLoopPromise<Void>?, ChannelPipeline) -> Void) {
self.eventLoop.assertInEventLoop()

switch (self.currentState, event) {
// origin: .fresh
case (.fresh, .beginRegistration):
return self.doStateTransfer(newState: .preRegistered, promise: promise) { pipeline in
self.currentState = .preRegistered
return { promise, pipeline in
promise?.succeed(result: ())
pipeline.fireChannelRegistered0()
}

case (.fresh, .close):
return self.doStateTransfer(newState: .closed, promise: promise) { (_: ChannelPipeline) in }
self.currentState = .closed
return { (promise, _: ChannelPipeline) in
promise?.succeed(result: ())
}

// origin: .preRegistered
case (.preRegistered, .finishRegistration):
return self.doStateTransfer(newState: .fullyRegistered, promise: promise) { pipeline in
// we don't tell the user about this
self.currentState = .fullyRegistered
return { (promise, _: ChannelPipeline) in
promise?.succeed(result: ())
}

// origin: .fullyRegistered
case (.fullyRegistered, .activate):
return self.doStateTransfer(newState: .activated, promise: promise) { pipeline in
self.currentState = .activated
return { promise, pipeline in
promise?.succeed(result: ())
pipeline.fireChannelActive0()
}

// origin: .preRegistered || .fullyRegistered
case (.preRegistered, .close), (.fullyRegistered, .close):
return self.doStateTransfer(newState: .closed, promise: promise) { pipeline in
self.currentState = .closed
return { promise, pipeline in
promise?.succeed(result: ())
pipeline.fireChannelUnregistered0()
}

// origin: .activated
case (.activated, .close):
return self.doStateTransfer(newState: .closed, promise: promise) { pipeline in
self.currentState = .closed
return { promise, pipeline in
promise?.succeed(result: ())
pipeline.fireChannelInactive0()
pipeline.fireChannelUnregistered0()
}
Expand All @@ -143,17 +155,7 @@ private struct SocketChannelLifecycleManager {
}

private func badTransition(event: Event) -> Never {
fatalError("illegal transition: state=\(self.currentState), event=\(event)")
}

@inline(__always) // we need to return a closure here and to not suffer from a potential allocation for that this must be inlined
private mutating func doStateTransfer(newState: State, promise: EventLoopPromise<Void>?, _ callouts: @escaping (ChannelPipeline) -> Void) -> ((ChannelPipeline) -> Void) {
self.currentState = newState

return { pipeline in
promise?.succeed(result: ())
callouts(pipeline)
}
preconditionFailure("illegal transition: state=\(self.currentState), event=\(event)")
}

// MARK: convenience properties
Expand Down Expand Up @@ -732,7 +734,7 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {
}

// Transition our internal state.
let callouts = self.lifecycleManager.close(promise: p)
let callouts = self.lifecycleManager.close()

// === END: No user callouts (now that our state is reconciled, we can call out to user code.) ===

Expand All @@ -750,7 +752,7 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {
connectPromise.fail(error: error)
}

callouts(self.pipeline)
callouts(p, self.pipeline)

eventLoop.execute {
// ensure this is executed in a delayed fashion as the users code may still traverse the pipeline
Expand Down Expand Up @@ -788,7 +790,7 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {
}

// we can't fully register yet as epoll would give us EPOLLHUP if bind/connect wasn't called yet.
self.lifecycleManager.beginRegistration(promise: promise)(self.pipeline)
self.lifecycleManager.beginRegistration()(promise, self.pipeline)
}

public final func registerAlreadyConfigured0(promise: EventLoopPromise<Void>?) {
Expand Down Expand Up @@ -1132,7 +1134,7 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {

// We always register with interested .none and will just trigger readIfNeeded0() later to re-register if needed.
try self.safeRegister(interested: [.readEOF, .reset])
self.lifecycleManager.finishRegistration(promise: nil)(self.pipeline)
self.lifecycleManager.finishRegistration()(nil, self.pipeline)
}

final func becomeActive0(promise: EventLoopPromise<Void>?) {
Expand All @@ -1146,7 +1148,7 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {
return
}
}
self.lifecycleManager.activate(promise: promise)(self.pipeline)
self.lifecycleManager.activate()(promise, self.pipeline)
self.readIfNeeded0()
}
}

0 comments on commit c66386e

Please sign in to comment.