code review

Author: hamzahrmalik

Sources/NIOCore/ByteBuffer-binaryEncodedLengthPrefix.swift

@@ -28,96 +28,104 @@ import Bionic
 
 /// Describes a way to encode and decode an integer as bytes
 ///
-public protocol BinaryIntegerEncodingStrategy {
-    /// Read an integer from a buffer. The reader index should be moved accordingly
-    /// - Returns: The integer
+public protocol NIOBinaryIntegerEncodingStrategy {
+    /// Read an integer from a buffer.
+    /// If there are not enough bytes to read an integer of this encoding, return nil, and do not move the reader index.
+    /// If the the full integer can be read, move the reader index to after the integer, and return the integer.
+    /// - Parameters:
+    ///   - as: The type of integer to be read.
+    ///   - buffer: The buffer to read from.
+    /// - Returns: The integer that was read, or nil if it was not possible to read it.
     func readInteger<IntegerType: FixedWidthInteger>(
         as: IntegerType.Type,
         from buffer: inout ByteBuffer
     ) -> IntegerType?
 
-    /// Write an integer to a buffer. The writer index should be moved accordingly
+    /// Write an integer to a buffer. Move the writer index to after the written integer.
+    /// - Parameters:
+    ///    - integer: The type of the integer to write.
+    ///    - to: The buffer to write to.
     func writeInteger<IntegerType: FixedWidthInteger>(
         _ integer: IntegerType,
         to buffer: inout ByteBuffer
     ) -> Int
 
-    /// When writing an integer using this strategy, how many bytes we should reserve
-    /// If the actual bytes used by the write function is more or less than this, it will be necessary to shuffle bytes
-    /// Therefore, an accurate prediction here will improve performance
-    var reservedSpaceForInteger: Int { get }
+    /// When writing an integer using this strategy, how many bytes we should reserve.
+    /// If the actual bytes used by the write function is more or less than this, it will be necessary to shuffle bytes.
+    /// Therefore, an accurate prediction here will improve performance.
+    var reservedCapacityForInteger: Int { get }
 
-    /// Write an integer to a buffer. The writer index should be moved accordingly
-    /// This function takes  a `reservedSpace` parameter which is the number of bytes we already reserved for writing this integer
-    /// If you use write more or less than this many bytes, we wll need to move bytes around to make that possible
-    /// Therefore, you may decide to encode differently to use more bytes than you actually would need, if it is possible for you to do so
-    /// That way, you waste the reserved bytes, but improve writing performance
-    func writeIntegerWithReservedSpace(
+    /// Write an integer to a buffer. The writer index should be moved accordingly.
+    /// This function takes  a `reservedCapacity` parameter which is the number of bytes we already reserved for writing this integer.
+    /// If you use write more or less than this many bytes, we wll need to move bytes around to make that possible.
+    /// Therefore, you may decide to encode differently to use more bytes than you actually would need, if it is possible for you to do so.
+    /// That way, you waste the reserved bytes, but improve writing performance.
+    func writeIntegerWithReservedCapacity(
         _ integer: Int,
-        reservedSpace: Int,
+        reservedCapacity: Int,
         to buffer: inout ByteBuffer
     ) -> Int
 }
 
-extension BinaryIntegerEncodingStrategy {
+extension NIOBinaryIntegerEncodingStrategy {
     @inlinable
-    public var reservedSpaceForInteger: Int { 1 }
+    public var reservedCapacityForInteger: Int { 1 }
 
     @inlinable
-    public func writeIntegerWithReservedSpace<IntegerType: FixedWidthInteger>(
+    public func writeIntegerWithReservedCapacity<IntegerType: FixedWidthInteger>(
         _ integer: IntegerType,
-        reservedSpace: Int,
+        reservedCapacity: Int,
         to buffer: inout ByteBuffer
     ) -> Int {
         self.writeInteger(integer, to: &buffer)
     }
 }
 
 extension ByteBuffer {
-    /// Read a binary encoded integer, moving the `readerIndex` appropriately
-    /// If there are not enough bytes, nil is returned
+    /// Read a binary encoded integer, moving the `readerIndex` appropriately.
+    /// If there are not enough bytes, nil is returned.
     @inlinable
-    public mutating func readEncodedInteger<Strategy: BinaryIntegerEncodingStrategy>(_ strategy: Strategy) -> Int? {
+    public mutating func readEncodedInteger<Strategy: NIOBinaryIntegerEncodingStrategy>(_ strategy: Strategy) -> Int? {
         strategy.readInteger(as: Int.self, from: &self)
     }
 
     /// Write a binary encoded integer.
     ///
-    /// - Returns: The number of bytes written
+    /// - Returns: The number of bytes written.
     @discardableResult
     @inlinable
     public mutating func writeEncodedInteger<
         Integer: FixedWidthInteger,
-        Strategy: BinaryIntegerEncodingStrategy
+        Strategy: NIOBinaryIntegerEncodingStrategy
     >(
         _ value: Integer,
         strategy: Strategy
     ) -> Int {
         strategy.writeInteger(value, to: &self)
     }
 
-    /// Prefixes a message written by `writeMessage` with the number of bytes written using `strategy`
+    /// Prefixes a message written by `writeMessage` with the number of bytes written using `strategy`.
     ///
-    /// Implementation note: This function works by reserving the number of bytes suggested by ``BinaryIntegerEncodingStrategy.reservedSpace`` before the data
-    /// It then writes the data, and then goes back to write the length
-    /// If the reserved capacity turns out to be too little or too much, then the data will be shifted
-    /// Therefore, this function is most performant if the strategy is able to use the same number of bytes that it reserved
+    /// - Note: This function works by reserving the number of bytes suggested by `strategy` before the data.
+    /// It then writes the data, and then goes back to write the length.
+    /// If the reserved capacity turns out to be too little or too much, then the data will be shifted.
+    /// Therefore, this function is most performant if the strategy is able to use the same number of bytes that it reserved.
     ///
     /// - Parameters:
-    ///     - strategy: The strategy to use for encoding the length
-    ///     - writeMessage: A closure that takes a buffer, writes a message to it and returns the number of bytes written
-    /// - Returns: Number of total bytes written. This is the length of the written message + the number of bytes used to write the length before it
+    ///     - strategy: The strategy to use for encoding the length.
+    ///     - writeMessage: A closure that takes a buffer, writes a message to it and returns the number of bytes written.
+    /// - Returns: Number of total bytes written. This is the length of the written message + the number of bytes used to write the length before it.
     @discardableResult
     @inlinable
-    public mutating func writeLengthPrefixed<Strategy: BinaryIntegerEncodingStrategy>(
+    public mutating func writeLengthPrefixed<Strategy: NIOBinaryIntegerEncodingStrategy>(
         strategy: Strategy,
         writeMessage: (inout ByteBuffer) throws -> Int
     ) rethrows -> Int {
         /// The index at which we write the length
         let lengthPrefixIndex = self.writerIndex
         /// The space which we reserve for writing the length
-        let reservedSpace = strategy.reservedSpaceForInteger
-        self.writeRepeatingByte(0, count: reservedSpace)
+        let reservedCapacity = strategy.reservedCapacityForInteger
+        self.writeRepeatingByte(0, count: reservedCapacity)
 
         /// The index at which we start writing the message originally. We may later move the message if the reserved space for the length wasn't right
         let originalMessageStartIndex = self.writerIndex
@@ -142,18 +150,18 @@ extension ByteBuffer {
         // We write the length after the message to begin with. We will move it later
 
         /// The actual number of bytes used to write the length written. The user may write more or fewer bytes than what we reserved
-        let actualIntegerLength = strategy.writeIntegerWithReservedSpace(
+        let actualIntegerLength = strategy.writeIntegerWithReservedCapacity(
             messageLength,
-            reservedSpace: reservedSpace,
+            reservedCapacity: reservedCapacity,
             to: &self
         )
 
         switch actualIntegerLength {
-        case reservedSpace:
+        case reservedCapacity:
             // Good, exact match, swap the values and then "delete" the trailing bytes by moving the index back
             self._moveBytes(from: originalMessageEndIndex, to: lengthPrefixIndex, size: actualIntegerLength)
             self.moveWriterIndex(to: originalMessageEndIndex)
-        case ..<reservedSpace:
+        case ..<reservedCapacity:
             // We wrote fewer bytes. We now have to move the length bytes from the end, and
             // _then_ shrink the rest of the buffer onto it.
             self._moveBytes(from: originalMessageEndIndex, to: lengthPrefixIndex, size: actualIntegerLength)
@@ -164,10 +172,10 @@ extension ByteBuffer {
                 size: messageLength
             )
             self.moveWriterIndex(to: newMessageStartIndex + messageLength)
-        case reservedSpace...:
+        case reservedCapacity...:
             // We wrote more bytes. We now have to create enough space. Once we do, we have the same
             // implementation as the matching case.
-            let extraSpaceNeeded = actualIntegerLength - reservedSpace
+            let extraSpaceNeeded = actualIntegerLength - reservedCapacity
             self._createSpace(before: lengthPrefixIndex, requiredSpace: extraSpaceNeeded)
 
             // Clean up the indices.
@@ -183,10 +191,10 @@ extension ByteBuffer {
         return totalBytesWritten
     }
 
-    /// Reads a slice which is prefixed with a length. The length will be read using `strategy`, and then that many bytes will be read to create a slice
-    /// - Returns: The slice, if there are enough bytes to read it fully. In this case, the readerIndex will move to after the slice
-    /// If there are not enough bytes to read the full slice, the readerIndex will stay unchanged
-    public mutating func readLengthPrefixedSlice<Strategy: BinaryIntegerEncodingStrategy>(
+    /// Reads a slice which is prefixed with a length. The length will be read using `strategy`, and then that many bytes will be read to create a slice.
+    /// - Returns: The slice, if there are enough bytes to read it fully. In this case, the readerIndex will move to after the slice.
+    /// If there are not enough bytes to read the full slice, the readerIndex will stay unchanged.
+    public mutating func readLengthPrefixedSlice<Strategy: NIOBinaryIntegerEncodingStrategy>(
         _ strategy: Strategy
     ) -> ByteBuffer? {
         let originalReaderIndex = self.readerIndex
@@ -202,14 +210,14 @@ extension ByteBuffer {
 // MARK: - Helpers for writing length-prefixed things
 
 extension ByteBuffer {
-    /// Write the length of `buffer` using `strategy`. Then write the buffer
+    /// Write the length of `buffer` using `strategy`. Then write the buffer.
     /// - Parameters:
-    ///   - buffer: The buffer to be written
-    ///   - strategy: The encoding strategy to use
-    /// - Returns: The total bytes written. This is the bytes needed to write the length, plus the length of the buffer itself
+    ///   - buffer: The buffer to be written.
+    ///   - strategy: The encoding strategy to use.
+    /// - Returns: The total bytes written. This is the bytes needed to write the length, plus the length of the buffer itself.
     @discardableResult
     public mutating func writeLengthPrefixedBuffer<
-        Strategy: BinaryIntegerEncodingStrategy
+        Strategy: NIOBinaryIntegerEncodingStrategy
     >(
         _ buffer: ByteBuffer,
         strategy: Strategy
@@ -220,14 +228,14 @@ extension ByteBuffer {
         return written
     }
 
-    /// Write the length of `string` using `strategy`. Then write the string
+    /// Write the length of `string` using `strategy`. Then write the string.
     /// - Parameters:
-    ///  - string: The string to be written
-    ///  - strategy: The encoding strategy to use
-    /// - Returns: The total bytes written. This is the bytes needed to write the length, plus the length of the string itself
+    ///  - string: The string to be written.
+    ///  - strategy: The encoding strategy to use.
+    /// - Returns: The total bytes written. This is the bytes needed to write the length, plus the length of the string itself.
     @discardableResult
     public mutating func writeLengthPrefixedString<
-        Strategy: BinaryIntegerEncodingStrategy
+        Strategy: NIOBinaryIntegerEncodingStrategy
     >(
         _ string: String,
         strategy: Strategy
@@ -240,15 +248,15 @@ extension ByteBuffer {
         return written
     }
 
-    /// Write the length of `bytes` using `strategy`. Then write the bytes
+    /// Write the length of `bytes` using `strategy`. Then write the bytes.
     /// - Parameters:
-    ///  - bytes: The bytes to be written
-    ///  - strategy: The encoding strategy to use
-    /// - Returns: The total bytes written. This is the bytes needed to write the length, plus the length of the bytes themselves
+    ///  - bytes: The bytes to be written.
+    ///  - strategy: The encoding strategy to use.
+    /// - Returns: The total bytes written. This is the bytes needed to write the length, plus the length of the bytes themselves.
     @inlinable
     public mutating func writeLengthPrefixedBytes<
         Bytes: Sequence,
-        Strategy: BinaryIntegerEncodingStrategy
+        Strategy: NIOBinaryIntegerEncodingStrategy
     >(
         _ bytes: Bytes,
         strategy: Strategy
@@ -271,11 +279,11 @@ extension ByteBuffer {
 }
 
 extension ByteBuffer {
-    /// Creates `requiredSpace` bytes of free space immediately before `index`
+    /// Creates `requiredSpace` bytes of free space immediately before `index`.
     /// e.g. given [a, b, c, d, e, f, g, h, i, j] and calling this function with (before: 4, requiredSpace: 2) would result in
     /// [a, b, c, d, 0, 0, e, f, g, h, i, j]
-    /// 2 extra bytes of space were created before index 4 (the letter e)
-    /// The total bytes written will be equal to `requiredSpace`, and the writer index will be moved accordingly
+    /// 2 extra bytes of space were created before index 4 (the letter e).
+    /// The total bytes written will be equal to `requiredSpace`, and the writer index will be moved accordingly.
     @usableFromInline
     mutating func _createSpace(before index: Int, requiredSpace: Int) {
         let bytesToMove = self.writerIndex - index
@@ -294,8 +302,8 @@ extension ByteBuffer {
         }
     }
 
-    /// Move the `size` bytes starting from `source` to `destination`
-    /// `source` and `destination` must both be within the writable range
+    /// Move the `size` bytes starting from `source` to `destination`.
+    /// `source` and `destination` must both be within the writable range.
     @usableFromInline
     mutating func _moveBytes(from source: Int, to destination: Int, size: Int) {
         precondition(source >= self.readerIndex && destination < self.writerIndex && source >= destination)

Sources/NIOCore/ByteBuffer-quicBinaryEncodingStrategy.swift

@@ -13,19 +13,19 @@
 //===----------------------------------------------------------------------===//
 
 extension ByteBuffer {
-    public struct QUICBinaryEncodingStrategy: BinaryIntegerEncodingStrategy {
-        public var reservedSpace: Int
+    public struct QUICBinaryEncodingStrategy: NIOBinaryIntegerEncodingStrategy {
+        public var reservedCapacityForInteger: Int
 
         @inlinable
-        public init(reservedSpace: Int) {
+        public init(reservedCapacity: Int) {
             precondition(
-                reservedSpace == 0
-                    || reservedSpace == 1
-                    || reservedSpace == 2
-                    || reservedSpace == 4
-                    || reservedSpace == 8
+                reservedCapacity == 0
+                    || reservedCapacity == 1
+                    || reservedCapacity == 2
+                    || reservedCapacity == 4
+                    || reservedCapacity == 8
             )
-            self.reservedSpace = reservedSpace
+            self.reservedCapacityForInteger = reservedCapacity
         }
 
         @inlinable
@@ -83,25 +83,22 @@ extension ByteBuffer {
             _ integer: IntegerType,
             to buffer: inout ByteBuffer
         ) -> Int {
-            self.writeIntegerWithReservedSpace(integer, reservedSpace: 0, to: &buffer)
+            self.writeIntegerWithReservedCapacity(integer, reservedCapacity: 0, to: &buffer)
         }
 
         @inlinable
-        public var reservedSpaceForInteger: Int { 4 }
-
-        @inlinable
-        public func writeIntegerWithReservedSpace<IntegerType: FixedWidthInteger>(
+        public func writeIntegerWithReservedCapacity<IntegerType: FixedWidthInteger>(
             _ integer: IntegerType,
-            reservedSpace: Int,
+            reservedCapacity: Int,
             to buffer: inout ByteBuffer
         ) -> Int {
-            if reservedSpace > 8 {
+            if reservedCapacity > 8 {
                 fatalError("Reserved space for QUIC encoded integer must be at most 8 bytes")
             }
             // Use more space than necessary in order to fill the reserved space
             // This will avoid a memmove
             // If the needed space is more than the reserved, we can't avoid the move
-            switch max(reservedSpace, self.bytesNeededForInteger(integer)) {
+            switch max(reservedCapacity, self.bytesNeededForInteger(integer)) {
             case 1:
                 // Easy, store the value. The top two bits are 0 so we don't need to do any masking.
                 return buffer.writeInteger(UInt8(truncatingIfNeeded: integer))
@@ -124,12 +121,12 @@ extension ByteBuffer {
     }
 }
 
-extension BinaryIntegerEncodingStrategy where Self == ByteBuffer.QUICBinaryEncodingStrategy {
+extension NIOBinaryIntegerEncodingStrategy where Self == ByteBuffer.QUICBinaryEncodingStrategy {
     @inlinable
-    public static func quic(reservedSpace: Int) -> ByteBuffer.QUICBinaryEncodingStrategy {
-        ByteBuffer.QUICBinaryEncodingStrategy(reservedSpace: reservedSpace)
+    public static func quic(reservedCapacity: Int) -> ByteBuffer.QUICBinaryEncodingStrategy {
+        ByteBuffer.QUICBinaryEncodingStrategy(reservedCapacity: reservedCapacity)
     }
 
     @inlinable
-    public static var quic: ByteBuffer.QUICBinaryEncodingStrategy { .quic(reservedSpace: 4) }
+    public static var quic: ByteBuffer.QUICBinaryEncodingStrategy { .quic(reservedCapacity: 4) }
 }