Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pre-box some errors to reduce allocations #2765

Merged
merged 5 commits into from
Jul 5, 2024
Merged

Pre-box some errors to reduce allocations #2765

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
416e701
Pre-box some errors to reduce allocations
glbrntt Jul 3, 2024
a565983
fix typo
glbrntt Jul 4, 2024
2c76206
Update alloc limits
glbrntt Jul 4, 2024
76fd253
Missed one
glbrntt Jul 5, 2024
3e1af8c
Merge branch 'main' into avoid-error-allocs
glbrntt Jul 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion Sources/NIOCore/Channel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ extension Channel {
}

public func registerAlreadyConfigured0(promise: EventLoopPromise<Void>?) {
promise?.fail(ChannelError.operationUnsupported)
promise?.fail(ChannelError._operationUnsupported)
}

public func triggerUserOutboundEvent(_ event: Any, promise: EventLoopPromise<Void>?) {
Expand Down Expand Up @@ -373,6 +373,16 @@ public enum ChannelError: Error {
case unremovableHandler
}

extension ChannelError {
// 'any Error' is unconditionally boxed, avoid allocating per use by statically boxing them.
static let _alreadyClosed: any Error = ChannelError.alreadyClosed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if should file a compiler bug here and this is something that the compiler ought to be able to optimise. The error enum is in the same module and it has zero associated values. So it might as well store the instance in a global memory region

Copy link
Contributor Author

@glbrntt glbrntt Jul 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's about the underlying error type; holding a ChannelError doesn't allocate. It's holding an any Error which always allocates. IIUC any Error has always been effectively just a pointer.

static let _inputClosed: any Error = ChannelError.inputClosed
static let _ioOnClosedChannel: any Error = ChannelError.ioOnClosedChannel
static let _operationUnsupported: any Error = ChannelError.operationUnsupported
static let _outputClosed: any Error = ChannelError.outputClosed
static let _unremovableHandler: any Error = ChannelError.unremovableHandler
}

extension ChannelError: Equatable { }

/// The removal of a `ChannelHandler` using `ChannelPipeline.removeHandler` has been attempted more than once.
Expand Down
44 changes: 22 additions & 22 deletions Sources/NIOCore/ChannelPipeline.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public final class ChannelPipeline: ChannelInvoker {
self.eventLoop.assertInEventLoop()

if self.destroyed {
return .failure(ChannelError.ioOnClosedChannel)
return .failure(ChannelError._ioOnClosedChannel)
}

switch position {
Expand Down Expand Up @@ -247,7 +247,7 @@ public final class ChannelPipeline: ChannelInvoker {
operation: (ChannelHandlerContext, ChannelHandlerContext) -> Void) -> Result<Void, Error> {
self.eventLoop.assertInEventLoop()
if self.destroyed {
return .failure(ChannelError.ioOnClosedChannel)
return .failure(ChannelError._ioOnClosedChannel)
}

guard let context = self.contextForPredicate0({ $0.handler === relativeHandler }) else {
Expand Down Expand Up @@ -280,7 +280,7 @@ public final class ChannelPipeline: ChannelInvoker {
self.eventLoop.assertInEventLoop()

if self.destroyed {
return .failure(ChannelError.ioOnClosedChannel)
return .failure(ChannelError._ioOnClosedChannel)
}

let context = ChannelHandlerContext(name: name ?? nextName(), handler: handler, pipeline: self)
Expand Down Expand Up @@ -416,7 +416,7 @@ public final class ChannelPipeline: ChannelInvoker {
/// - promise: An `EventLoopPromise` that will complete when the `ChannelHandler` is removed.
public func removeHandler(context: ChannelHandlerContext, promise: EventLoopPromise<Void>?) {
guard context.handler is RemovableChannelHandler else {
promise?.fail(ChannelError.unremovableHandler)
promise?.fail(ChannelError._unremovableHandler)
return
}
func removeHandler0() {
Expand Down Expand Up @@ -826,7 +826,7 @@ public final class ChannelPipeline: ChannelInvoker {
if let firstOutboundCtx = firstOutboundCtx {
firstOutboundCtx.invokeClose(mode: mode, promise: promise)
} else {
promise?.fail(ChannelError.alreadyClosed)
promise?.fail(ChannelError._alreadyClosed)
}
}

Expand All @@ -846,47 +846,47 @@ public final class ChannelPipeline: ChannelInvoker {
if let firstOutboundCtx = firstOutboundCtx {
firstOutboundCtx.invokeWrite(data, promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

private func writeAndFlush0(_ data: NIOAny, promise: EventLoopPromise<Void>?) {
if let firstOutboundCtx = firstOutboundCtx {
firstOutboundCtx.invokeWriteAndFlush(data, promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

private func bind0(to address: SocketAddress, promise: EventLoopPromise<Void>?) {
if let firstOutboundCtx = firstOutboundCtx {
firstOutboundCtx.invokeBind(to: address, promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

private func connect0(to address: SocketAddress, promise: EventLoopPromise<Void>?) {
if let firstOutboundCtx = firstOutboundCtx {
firstOutboundCtx.invokeConnect(to: address, promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

private func register0(promise: EventLoopPromise<Void>?) {
if let firstOutboundCtx = firstOutboundCtx {
firstOutboundCtx.invokeRegister(promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

private func triggerUserOutboundEvent0(_ event: Any, promise: EventLoopPromise<Void>?) {
if let firstOutboundCtx = firstOutboundCtx {
firstOutboundCtx.invokeTriggerUserOutboundEvent(event, promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

Expand Down Expand Up @@ -1378,14 +1378,14 @@ extension ChannelPipeline.Position: Sendable {}

private extension CloseMode {
/// Returns the error to fail outstanding operations writes with.
var error: ChannelError {
var error: any Error {
switch self {
case .all:
return .ioOnClosedChannel
return ChannelError._ioOnClosedChannel
case .output:
return .outputClosed
return ChannelError._outputClosed
case .input:
return .inputClosed
return ChannelError._inputClosed
}
}
}
Expand Down Expand Up @@ -1577,7 +1577,7 @@ public final class ChannelHandlerContext: ChannelInvoker {
if let outboundNext = self.prev {
outboundNext.invokeRegister(promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

Expand All @@ -1591,7 +1591,7 @@ public final class ChannelHandlerContext: ChannelInvoker {
if let outboundNext = self.prev {
outboundNext.invokeBind(to: address, promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

Expand All @@ -1605,7 +1605,7 @@ public final class ChannelHandlerContext: ChannelInvoker {
if let outboundNext = self.prev {
outboundNext.invokeConnect(to: address, promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

Expand All @@ -1620,7 +1620,7 @@ public final class ChannelHandlerContext: ChannelInvoker {
if let outboundNext = self.prev {
outboundNext.invokeWrite(data, promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

Expand All @@ -1647,7 +1647,7 @@ public final class ChannelHandlerContext: ChannelInvoker {
if let outboundNext = self.prev {
outboundNext.invokeWriteAndFlush(data, promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

Expand All @@ -1671,7 +1671,7 @@ public final class ChannelHandlerContext: ChannelInvoker {
if let outboundNext = self.prev {
outboundNext.invokeClose(mode: mode, promise: promise)
} else {
promise?.fail(ChannelError.alreadyClosed)
promise?.fail(ChannelError._alreadyClosed)
}
}

Expand All @@ -1684,7 +1684,7 @@ public final class ChannelHandlerContext: ChannelInvoker {
if let outboundNext = self.prev {
outboundNext.invokeTriggerUserOutboundEvent(event, promise: promise)
} else {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}
}

Expand Down
22 changes: 11 additions & 11 deletions Sources/NIOCore/DeadChannel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,31 @@
/// all operations.
private final class DeadChannelCore: ChannelCore {
func localAddress0() throws -> SocketAddress {
throw ChannelError.ioOnClosedChannel
throw ChannelError._ioOnClosedChannel
}

func remoteAddress0() throws -> SocketAddress {
throw ChannelError.ioOnClosedChannel
throw ChannelError._ioOnClosedChannel
}

func register0(promise: EventLoopPromise<Void>?) {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}

func registerAlreadyConfigured0(promise: EventLoopPromise<Void>?) {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}

func bind0(to: SocketAddress, promise: EventLoopPromise<Void>?) {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}

func connect0(to: SocketAddress, promise: EventLoopPromise<Void>?) {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}

func write0(_ data: NIOAny, promise: EventLoopPromise<Void>?) {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}

func flush0() {
Expand All @@ -51,11 +51,11 @@ private final class DeadChannelCore: ChannelCore {
}

func close0(error: Error, mode: CloseMode, promise: EventLoopPromise<Void>?) {
promise?.fail(ChannelError.alreadyClosed)
promise?.fail(ChannelError._alreadyClosed)
}

func triggerUserOutboundEvent0(_ event: Any, promise: EventLoopPromise<Void>?) {
promise?.fail(ChannelError.ioOnClosedChannel)
promise?.fail(ChannelError._ioOnClosedChannel)
}

func channelRead0(_ data: NIOAny) {
Expand Down Expand Up @@ -104,11 +104,11 @@ internal final class DeadChannel: Channel, @unchecked Sendable {
let parent: Channel? = nil

func setOption<Option: ChannelOption>(_ option: Option, value: Option.Value) -> EventLoopFuture<Void> {
return self.pipeline.eventLoop.makeFailedFuture(ChannelError.ioOnClosedChannel)
return self.pipeline.eventLoop.makeFailedFuture(ChannelError._ioOnClosedChannel)
}

func getOption<Option: ChannelOption>(_ option: Option) -> EventLoopFuture<Option.Value> {
return eventLoop.makeFailedFuture(ChannelError.ioOnClosedChannel)
return eventLoop.makeFailedFuture(ChannelError._ioOnClosedChannel)
}

let isWritable = false
Expand Down
9 changes: 7 additions & 2 deletions Sources/NIOCore/EventLoop.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public struct Scheduled<T> {
@usableFromInline typealias CancelationCallback = @Sendable () -> Void
/* private but usableFromInline */ @usableFromInline let _promise: EventLoopPromise<T>
/* private but usableFromInline */ @usableFromInline let _cancellationTask: CancelationCallback

@inlinable
@preconcurrency
public init(promise: EventLoopPromise<T>, cancellationTask: @escaping @Sendable () -> Void) {
Expand All @@ -40,7 +40,7 @@ public struct Scheduled<T> {
/// This means that cancellation is not guaranteed.
@inlinable
public func cancel() {
self._promise.fail(EventLoopError.cancelled)
self._promise.fail(EventLoopError._cancelled)
self._cancellationTask()
}

Expand Down Expand Up @@ -1219,6 +1219,11 @@ public enum EventLoopError: Error {
case shutdownFailed
}

extension EventLoopError {
@usableFromInline
static let _cancelled: any Error = EventLoopError.cancelled
}

extension EventLoopError: CustomStringConvertible {
public var description: String {
switch self {
Expand Down
8 changes: 4 additions & 4 deletions Sources/NIOPosix/BSDSocketAPIPosix.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ extension NIOBSDSocket {
option_value: &segmentSize,
option_len: socklen_t(MemoryLayout<CInt>.size))
#else
throw ChannelError.operationUnsupported
throw ChannelError._operationUnsupported
#endif
}

Expand All @@ -294,7 +294,7 @@ extension NIOBSDSocket {
}
return segmentSize
#else
throw ChannelError.operationUnsupported
throw ChannelError._operationUnsupported
#endif
}

Expand All @@ -307,7 +307,7 @@ extension NIOBSDSocket {
option_value: &isEnabled,
option_len: socklen_t(MemoryLayout<CInt>.size))
#else
throw ChannelError.operationUnsupported
throw ChannelError._operationUnsupported
#endif
}

Expand All @@ -324,7 +324,7 @@ extension NIOBSDSocket {
}
return enabled != 0
#else
throw ChannelError.operationUnsupported
throw ChannelError._operationUnsupported
#endif
}
}
Expand Down
Loading