ronantakizawa/codereview-bench huggingface dataset:
Repository: apple/swift-nio
File: Sources/NIOCore/ByteBuffer-conversions.swift (Swift)

Code before:
    public init(buffer: ByteBuffer) {
        var buffer = buffer
        self = buffer.readBytes(length: buffer.readableBytes)!
    }
    
}

extension String {
    
    /// Creates a `String` from a given `ByteBuffer`. The entire readable portion of the buffer will be read.
    /// - parameter buffer: The buffer to read.
    @inlinable
    public init(buffer: ByteBuffer) {
        var buffer = buffer
        self = buffer.readString(length: buffer.readableBytes)!
    }

    /// Creates a `String` from a given Int with base (radix) of 16, with the provided padding size.
    /// - parameter padding: The desired length of the resulting string.
    @inlinable
    public init<T>(byte: T, padding: Int) where T : BinaryInteger {
        self.init(byte, radix: 16)
        var hexString = String(byte, radix: 16)
        while hexString.count < padding {
            hexString = "0" + hexString
        }
        self = hexString
    }
}

extension DispatchData {
    
    /// Creates a `DispatchData` from a given `ByteBuffer`. The entire readable portion of the buffer will be read.
    /// - parameter buffer: The buffer to read.
    @inlinable
    public init(buffer: ByteBuffer) {
        var buffer = buffer
        self = buffer.readDispatchData(length: buffer.readableBytes)!
    }
    
}


Review comment (performance):
This is expensive! Instead calculate the amount of padding required and use `String(repeating: "0", count: amountOfPadding)` to create the padding and then combine it with the `hexString`

Code after:
    public init(buffer: ByteBuffer) {
        var buffer = buffer
        self = buffer.readBytes(length: buffer.readableBytes)!
    }
    
}

extension String {
    
    /// Creates a `String` from a given `ByteBuffer`. The entire readable portion of the buffer will be read.
    /// - parameter buffer: The buffer to read.
    @inlinable
    public init(buffer: ByteBuffer) {
        var buffer = buffer
        self = buffer.readString(length: buffer.readableBytes)!
    }

    /// Creates a `String` from a given `Int` with a given base (`radix`), padded with zeroes to the provided `padding` size.
    ///
    /// - parameters:
    ///     - radix: radix base to use for conversion.
    ///     - padding: the desired lenght of the resulting string.
    @inlinable
    internal init<Value>(_ value: Value, radix: Int, padding: Int) where Value: BinaryInteger {
        let formatted = String(value, radix: radix)
        self = String(repeating: "0", count: padding - formatted.count) + formatted
    }
}

extension DispatchData {
    
    /// Creates a `DispatchData` from a given `ByteBuffer`. The entire readable portion of the buffer will be read.
    /// - parameter buffer: The buffer to read.
    @inlinable
    public init(buffer: ByteBuffer) {
        var buffer = buffer
        self = buffer.readDispatchData(length: buffer.readableBytes)!
    }
    
}
Repository: apple/swift-nio
File: Sources/NIOCore/ByteBuffer-hexdump.swift (Swift)

Code before:
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2017-2023 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

extension ByteBuffer {

    /// Return a `String` of hexadecimal digits of bytes in the Buffer,
    /// in a format that's compatible with `xxd -r -p`.
    /// `hexDumpShort()` always dumps all readable bytes, i.e. from `readerIndex` to `writerIndex`,
    /// so you should set those indices to desired location to get the offset and length that you need to dump.
    func hexDumpShort() -> String {
        var hexString = ""
        hexString.reserveCapacity(self.readableBytes*3)

        for byte in self.readableBytesView {
            hexString += "\(String(byte: byte, padding: 2)) "
        }

        return String(hexString.dropLast())
    }

Review comment (security):
This is a new file so the year should just be 2023:

```suggestion
// Copyright (c) 2023 Apple Inc. and the SwiftNIO project authors
```

Code after:
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2023 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

extension ByteBuffer {

    /// Describes a ByteBuffer hexDump format.
    /// Can be either xxd output compatible, or hexdump compatible.
    public struct HexDumpFormat: Hashable, Sendable {

        enum Value: Hashable {
            case plain(maxBytes: Int? = nil)
            case detailed(maxBytes: Int? = nil)
        }

        let value: Value
        init(_ value: Value) { self.value = value }

        /// A plain hex dump format compatible with `xxd` CLI utility.
        public static let plain = Self(.plain(maxBytes: nil))
Repository: apple/swift-nio
File: Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift (Swift)

Code before:
// Copyright (c) 2023 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

import Benchmark

let benchmarks = {
    let defaultMetrics: [BenchmarkMetric] = [
        .mallocCountTotal,
    ]

    Benchmark(
        "TCPEcho",
        configuration: .init(
            metrics: defaultMetrics,
            timeUnits: .milliseconds,
            scalingFactor: .mega
        )
    ) { benchmark in
        try runTCPEcho(numberOfWrites: 10_000)
    }
}


Review comment (suggestion):
Scalingfactor 1M but writing 10K? Perhaps consider numberOfWrites to be scalingfactor if you want to make sense of output from --scale later. Otherwise probably skip setting scalingfactor here, as it's really intended for inner loops in practice. 

Code after:
// Copyright (c) 2023 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

import Benchmark

let benchmarks = {
    let defaultMetrics: [BenchmarkMetric] = [
        .mallocCountTotal,
    ]

    Benchmark(
        "TCPEcho",
        configuration: .init(
            metrics: defaultMetrics,
            timeUnits: .milliseconds,
            scalingFactor: .mega
        )
    ) { benchmark in
        try runTCPEcho(numberOfWrites: benchmark.scaledIterations.upperBound)
    }
}
Repository: apple/swift-nio
File: Tests/NIOHTTP1Tests/ContentLengthTests.swift (Swift)

Code before:
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2017-2021 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

import XCTest
import NIOCore
import NIOEmbedded
import NIOHTTP1

class ContentLengthTests: XCTestCase {

    /// Client receives a response longer than the content-length header
    func testResponseContentTooLong() throws {
        let channel = EmbeddedChannel()
        try channel.pipeline.syncOperations.addHTTPClientHandlers()
        defer {
            _ = try? channel.finish()
        }
        // Receive a response with a content-length header of 2 but a body of more than 2 bytes
        let badResponse = "HTTP/1.1 200 OK\r\nServer: foo\r\nContent-Length: 2\r\n\r\ntoo many bytes"

Review comment (security):
```suggestion
// Copyright (c) 2023 Apple Inc. and the SwiftNIO project authors
```

Code after:
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2023 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

import XCTest
import NIOCore
import NIOEmbedded
import NIOHTTP1

final class ContentLengthTests: XCTestCase {

    /// Client receives a response longer than the content-length header
    func testResponseContentTooLong() throws {
        let channel = EmbeddedChannel()
        try channel.pipeline.syncOperations.addHTTPClientHandlers()
        defer {
            _ = try? channel.finish()
        }
        // Receive a response with a content-length header of 2 but a body of more than 2 bytes
        let badResponse = "HTTP/1.1 200 OK\r\nServer: foo\r\nContent-Length: 2\r\n\r\ntoo many bytes"
Repository: apple/swift-nio
File: Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift (Swift)

Code before:
        .mallocCountTotal,
    ]

    Benchmark(
        "TCPEcho",
        configuration: .init(
            metrics: defaultMetrics,
            timeUnits: .milliseconds,
            scalingFactor: .mega
        )
    ) { benchmark in
        try runTCPEcho(
            numberOfWrites: benchmark.scaledIterations.upperBound,
            eventLoop: eventLoop
        )
    }

    // This benchmark is only available above 5.9 since our EL conformance
    // to serial executor is also gated behind 5.9.
    #if compiler(>=5.9)
    Benchmark(
        "TCPEchoAsyncChannel",
        configuration: .init(
            metrics: defaultMetrics,
            timeUnits: .milliseconds,
            scalingFactor: .mega,
            setup: {
                swiftTaskEnqueueGlobalHook = { job, _ in
                    eventLoop.executor.enqueue(job)
                }
            },
            teardown: {
                swiftTaskEnqueueGlobalHook = nil
            }
        )
    ) { benchmark in
        try await runTCPEchoAsyncChannel(
            numberOfWrites: benchmark.scaledIterations.upperBound,
            eventLoop: eventLoop
        )
    }
    #endif
}


Review comment (suggestion):
If you update to Benchmark `1.22.0` you can now add some slack to the p90 check for malloc counts, this should do the trick (allowing for a +-100 mallocs);

```suggestion
            scalingFactor: .mega,
            thresholds: [.mallocCountTotal : .init(absolute: [.p90 : 100])],
```


Code after:
        .mallocCountTotal,
    ]

    Benchmark(
        "TCPEcho",
        configuration: .init(
            metrics: defaultMetrics,
            timeUnits: .milliseconds,
            scalingFactor: .mega
        )
    ) { benchmark in
        try runTCPEcho(
            numberOfWrites: benchmark.scaledIterations.upperBound,
            eventLoop: eventLoop
        )
    }

    // This benchmark is only available above 5.9 since our EL conformance
    // to serial executor is also gated behind 5.9.
    #if compiler(>=5.9)
    Benchmark(
        "TCPEchoAsyncChannel",
        configuration: .init(
            metrics: defaultMetrics,
            timeUnits: .milliseconds,
            scalingFactor: .mega,
            // We are expecting a bit of allocation variance due to an allocation
            // in the Concurrency runtime which happens when resuming a continuation.
            thresholds: [.mallocCountTotal: .init(absolute: [.p90: 2000])],
            setup: {
                swiftTaskEnqueueGlobalHook = { job, _ in
                    eventLoop.executor.enqueue(job)
                }
            },
            teardown: {
                swiftTaskEnqueueGlobalHook = nil
            }
        )
    ) { benchmark in
        try await runTCPEchoAsyncChannel(
            numberOfWrites: benchmark.scaledIterations.upperBound,
            eventLoop: eventLoop
        )
    }
    #endif
}
Repository: apple/swift-nio
File: Package.swift (Swift)

Code before:
                swiftCollections
            ]
        ),
        .target(
            name: "NIOWebSocket",
            dependencies: [
                "NIO",
                "NIOCore",
                "NIOHTTP1",
                "CNIOSHA1",
                "_NIOBase64"
            ]
        ),
        .target(
            name: "CNIOLLHTTP",
            cSettings: [
              .define("_GNU_SOURCE"),
              .define("LLHTTP_STRICT_MODE")
            ]
        ),
        .target(
            name: "NIOTLS",
            dependencies: [
                "NIO",
                "NIOCore",
                swiftCollections,
            ]
        ),
        .target(
            name: "NIOTestUtils",
            dependencies: [
                "NIOPosix",
                "NIOCore",
                "NIOEmbedded",
                "NIOHTTP1",
                swiftAtomics,
            ]
        ),
        .target(
            name: "NIOFileSystem",
            dependencies: [
                "NIOCore",
                "CNIOLinux",
                "CNIODarwin",
                swiftAtomics,
                swiftCollections,
                swiftSystem,
            ],
            swiftSettings: [
                .define("ENABLE_MOCKING", .when(configuration: .debug))
            ]

Review comment (suggestion):
This condition should probably be persisted across almost any library target that depends on `NIO` which also depends on `NIOCore`.

`_NIOConcurrency`, `NIOFoundationCompat`, `NIOWebSocket`, `NIOTLS` appear to be the other places.

Code after:
        .target(
            name: "CNIODarwin",
            dependencies: [],
            cSettings: [
                .define("__APPLE_USE_RFC_3542")
            ]
        ),
        .target(
            name: "CNIOWindows",
            dependencies: []
        ),
        .target(
            name: "CNIOWASI",
            dependencies: []
        ),
        .target(
            name: "NIOConcurrencyHelpers",
            dependencies: [
                "CNIOAtomics"
            ],
            swiftSettings: strictConcurrencySettings
        ),
        .target(
            name: "NIOHTTP1",
            dependencies: [
                .target(name: "NIO", condition: .when(platforms: historicalNIOPosixDependencyRequired)),
                "NIOCore",
                "NIOConcurrencyHelpers",
                "CNIOLLHTTP",
                swiftCollections,
            ]
        ),
        .target(
            name: "NIOWebSocket",
            dependencies: [
                .target(name: "NIO", condition: .when(platforms: historicalNIOPosixDependencyRequired)),
                "NIOCore",
                "NIOHTTP1",
                "CNIOSHA1",
                "_NIOBase64",
            ]
        ),
        .target(
            name: "CNIOLLHTTP",
            cSettings: [
                .define("_GNU_SOURCE"),
                .define("LLHTTP_STRICT_MODE"),
            ]
        ),
        .target(
            name: "NIOTLS",
Repository: apple/swift-nio
File: Sources/NIOPosix/Bootstrap.swift (Swift)

Code before:
                    return channel.close(mode: .output).map { result }
                }
                return channel.selectableEventLoop.makeSucceededFuture(result)
            }.flatMapError { error in
                channel.close0(error: error, mode: .all, promise: nil)
                return channel.eventLoop.makeFailedFuture(error)
            }
        }

        if eventLoop.inEventLoop {
            return setupChannel()
        } else {
            return eventLoop.flatSubmit {
                setupChannel()
            }
        }
    }
}

@available(*, unavailable)
extension NIOPipeBootstrap: Sendable {}

protocol NIOPipeBootstrapHooks {
    func makePipeChannel(eventLoop: SelectableEventLoop,
                         inputPipe: NIOFileHandle?,
                         outputPipe: NIOFileHandle?) throws -> PipeChannel;
}

fileprivate struct DefaultNIOPipeBootstrapHooks : NIOPipeBootstrapHooks {
    func makePipeChannel(eventLoop: SelectableEventLoop,
                         inputPipe: NIOFileHandle?,
                         outputPipe: NIOFileHandle?) throws -> PipeChannel {
        return try PipeChannel(eventLoop: eventLoop, inputPipe: inputPipe, outputPipe: outputPipe)
    }
}


Review comment (suggestion):
```suggestion
                         outputPipe: NIOFileHandle?) throws -> PipeChannel
```

Code after:
                    return channel.close(mode: .output).map { result }
                }
                return channel.selectableEventLoop.makeSucceededFuture(result)
            }.flatMapError { error in
                channel.close0(error: error, mode: .all, promise: nil)
                return channel.eventLoop.makeFailedFuture(error)
            }
        }

        if eventLoop.inEventLoop {
            return setupChannel()
        } else {
            return eventLoop.flatSubmit {
                setupChannel()
            }
        }
    }
}

@available(*, unavailable)
extension NIOPipeBootstrap: Sendable {}

protocol NIOPipeBootstrapHooks {
    func makePipeChannel(eventLoop: SelectableEventLoop,
                         inputPipe: NIOFileHandle?,
                         outputPipe: NIOFileHandle?) throws -> PipeChannel
}

fileprivate struct DefaultNIOPipeBootstrapHooks: NIOPipeBootstrapHooks {
    func makePipeChannel(eventLoop: SelectableEventLoop,
                         inputPipe: NIOFileHandle?,
                         outputPipe: NIOFileHandle?) throws -> PipeChannel {
        return try PipeChannel(eventLoop: eventLoop, inputPipe: inputPipe, outputPipe: outputPipe)
    }
}
Repository: apple/swift-nio
File: Sources/NIOPosix/Bootstrap.swift (Swift)

Code before:
            }.flatMapError { error in
                channel.close0(error: error, mode: .all, promise: nil)
                return channel.eventLoop.makeFailedFuture(error)
            }
        }

        if eventLoop.inEventLoop {
            return setupChannel()
        } else {
            return eventLoop.flatSubmit {
                setupChannel()
            }
        }
    }
}

@available(*, unavailable)
extension NIOPipeBootstrap: Sendable {}

protocol NIOPipeBootstrapHooks {
    func makePipeChannel(eventLoop: SelectableEventLoop,
                         inputPipe: NIOFileHandle?,
                         outputPipe: NIOFileHandle?) throws -> PipeChannel;
}

fileprivate struct DefaultNIOPipeBootstrapHooks : NIOPipeBootstrapHooks {
    func makePipeChannel(eventLoop: SelectableEventLoop,
                         inputPipe: NIOFileHandle?,
                         outputPipe: NIOFileHandle?) throws -> PipeChannel {
        return try PipeChannel(eventLoop: eventLoop, inputPipe: inputPipe, outputPipe: outputPipe)
    }
}


Review comment (suggestion):
```suggestion
fileprivate struct DefaultNIOPipeBootstrapHooks: NIOPipeBootstrapHooks {
```

Code after:
            }.flatMapError { error in
                channel.close0(error: error, mode: .all, promise: nil)
                return channel.eventLoop.makeFailedFuture(error)
            }
        }

        if eventLoop.inEventLoop {
            return setupChannel()
        } else {
            return eventLoop.flatSubmit {
                setupChannel()
            }
        }
    }
}

@available(*, unavailable)
extension NIOPipeBootstrap: Sendable {}

protocol NIOPipeBootstrapHooks {
    func makePipeChannel(eventLoop: SelectableEventLoop,
                         inputPipe: NIOFileHandle?,
                         outputPipe: NIOFileHandle?) throws -> PipeChannel
}

fileprivate struct DefaultNIOPipeBootstrapHooks: NIOPipeBootstrapHooks {
    func makePipeChannel(eventLoop: SelectableEventLoop,
                         inputPipe: NIOFileHandle?,
                         outputPipe: NIOFileHandle?) throws -> PipeChannel {
        return try PipeChannel(eventLoop: eventLoop, inputPipe: inputPipe, outputPipe: outputPipe)
    }
}
Repository: apple/swift-nio
File: Sources/NIOCore/Codec.swift (Swift)

Code before:
        self.state = .done
        self.buffer = nil
    }

    public func write(context: ChannelHandlerContext, data: NIOAny, promise: EventLoopPromise<Void>?) {
        switch self.state {
        case .notInChannelYet:
            preconditionFailure("MessageToByteHandler.write called before it was added to a Channel")
        case .error(let error):
            promise?.fail(error)
            context.fireErrorCaught(error)
            return
        case .done:
            // let's just ignore this
            return
        case .operational:
            // there's actually some work to do here
            break
        }
        let data = self.unwrapOutboundIn(data)

        do {
            self.buffer!.clear()
            try self.encoder.encode(data: data, out: &self.buffer!)
            context.write(self.wrapOutboundOut(self.buffer!), promise: promise)
            if let maxBufferCapacity {
                self.buffer?.clampBufferCapacity(to: maxBufferCapacity)
            }
        } catch {
            self.state = .error(error)
            promise?.fail(error)
            context.fireErrorCaught(error)
        }
    }
}


Review comment (suggestion):
I am still a bit worried of doing this on every single write. Instead of adding some heuristic to only do It after a period of small writes.

Code after:
    private let desiredBufferCapacity: Int?

    public init(_ encoder: Encoder, desiredBufferCapacity: Int) {
        self.encoder = encoder
        self.desiredBufferCapacity = desiredBufferCapacity
    }

    public init(_ encoder: Encoder) {
        self.encoder = encoder
        self.desiredBufferCapacity = nil
    }
}

@available(*, unavailable)
extension MessageToByteHandler: Sendable {}

extension MessageToByteHandler {
    public func handlerAdded(context: ChannelHandlerContext) {
        precondition(
            self.state.readyToBeAddedToChannel,
            "illegal state when adding to Channel: \(self.state)"
        )
        self.state = .operational
        self.buffer = context.channel.allocator.buffer(capacity: 256)
    }

    public func handlerRemoved(context: ChannelHandlerContext) {
        self.state = .done
        self.buffer = nil
    }

    public func write(context: ChannelHandlerContext, data: NIOAny, promise: EventLoopPromise<Void>?) {
        switch self.state {
        case .notInChannelYet:
            preconditionFailure("MessageToByteHandler.write called before it was added to a Channel")
        case .error(let error):
            promise?.fail(error)
            context.fireErrorCaught(error)
            return
        case .done:
            // let's just ignore this
            return
        case .operational:
            // there's actually some work to do here
            break
        }
        let data = Self.unwrapOutboundIn(data)

        do {
            self.buffer!.clear()
            try self.encoder.encode(data: data, out: &self.buffer!)