Replace Data with SwiftProtobufContiguousBytes in serialization path (apple#1391)

Usages of `Data` in the serialisation path have been replaced by usages of the new `SwiftProtobufContiguousBytes` protocol so that `[UInt8]` can also be used in the serialisation path, avoiding unnecessary translations between arrays of bytes and `Data`.

Note: we have decided **not** to change the `bytes` field of the protos from `Data` to `[UInt8]`/`SwiftProtobufContiguousBytes`, so this change does not include this.

This PR includes some new `Data` allocations, which are temporary to avoid growing the PR too much. I tried to restrict my changes to just the serialisation path, and plan on sending another one for deserialisation and a third one for JSON encoding/decoding.

Author: gjcairo

Documentation/API.md

@@ -68,7 +68,7 @@ public struct Example: SwiftProtobuf.Message {
   // An extension map may be needed when decoding nested
   // proto2-format messages that utilize extensions.
   // See below for more details.
-  func serializedData() throws -> Data
+  func serializedBytes<Bytes: SwiftProtobufContiguousBytes>() throws -> Bytes
   init<Bytes: SwiftProtobufContiguousBytes>(serializedBytes: Bytes) throws {
   init<Bytes: SwiftProtobufContiguousBytes>(serializedBytes: Bytes, extensions: ExtensionMap? = nil, partial: Bool = false) throws
 

FuzzTesting/Sources/FuzzBinary/main.swift

@@ -1,5 +1,3 @@
-import Foundation
-
 import FuzzCommon
 
 import SwiftProtobuf
@@ -10,14 +8,14 @@ public func FuzzBinary(_ start: UnsafeRawPointer, _ count: Int) -> CInt {
   var msg: Fuzz_Testing_Message?
   do {
     msg = try Fuzz_Testing_Message(
-      serializedData: Data(bytes),
+      serializedBytes: Array(bytes),
       extensions: Fuzz_Testing_FuzzTesting_Extensions)
   } catch {
     // Error parsing are to be expected since not all input will be well formed.
   }
   // Test serialization for completeness.
   // If a message was parsed, it should not fail to serialize, so assert as such.
-  let _ = try! msg?.serializedData()
+  let _: [UInt8]? = try! msg?.serializedBytes()
 
   return 0
 }

Performance/generators/swift.sh

@@ -112,7 +112,7 @@ extension Harness {
 
       // Exercise binary serialization.
       let data = try measureSubtask("Encode binary") {
-        return try message.serializedData()
+        return try message.serializedBytes()
       }
       let message2 = try measureSubtask("Decode binary") {
         return try PerfMessage(serializedData: data)

README.md

@@ -261,12 +261,17 @@ let info2 = BookInfo.with {
     $0.author = "Jane Q. Smith"
   }
 
-// Serialize to binary protobuf format:
-let binaryData: Data = try info.serializedData()
+// Serialize to binary protobuf format: you can choose to serialize into
+// any type conforming to SwiftProtobufContiguousBytes. For example:
+let binaryData: Data = try info.serializedBytes()
+let binaryDataAsBytes: [UInt8] = try info.serializedBytes()
 
 // Deserialize a received Data object from `binaryData`
 let decodedInfo = try BookInfo(serializedData: binaryData)
 
+// Deserialize a received [UInt8] object from `binaryDataAsBytes`
+let decodedInfo = try BookInfo(serializedBytes: binaryDataAsBytes)
+
 // Serialize to JSON format as a Data object
 let jsonData: Data = try info.jsonUTF8Data()
 

Sources/Conformance/main.swift

@@ -21,10 +21,9 @@ import Glibc
 import Darwin.C
 #endif
 
-import Foundation
 import SwiftProtobuf
 
-func readRequest() -> Data? {
+func readRequest() -> [UInt8]? {
     var rawCount: UInt32 = 0
     let read1 = fread(&rawCount, 1, 4, stdin)
     let count = Int(rawCount)
@@ -36,11 +35,10 @@ func readRequest() -> Data? {
     if read2 < count {
         return nil
     }
-    return Data(buff)
+    return buff
 }
 
-func writeResponse(data: Data) {
-    let bytes = [UInt8](data)
+func writeResponse(bytes: [UInt8]) {
     var count = UInt32(bytes.count)
     fwrite(&count, 4, 1, stdout)
     _ = bytes.withUnsafeBufferPointer { bp in
@@ -49,12 +47,12 @@ func writeResponse(data: Data) {
     fflush(stdout)
 }
 
-func buildResponse(serializedData: Data) -> Conformance_ConformanceResponse {
+func buildResponse(serializedBytes: [UInt8]) -> Conformance_ConformanceResponse {
     var response = Conformance_ConformanceResponse()
 
     let request: Conformance_ConformanceRequest
     do {
-        request = try Conformance_ConformanceRequest(serializedData: serializedData)
+        request = try Conformance_ConformanceRequest(serializedBytes: serializedBytes)
     } catch {
         response.runtimeError = "Failed to parse conformance request"
         return response
@@ -142,7 +140,7 @@ func buildResponse(serializedData: Data) -> Conformance_ConformanceResponse {
     switch request.requestedOutputFormat {
     case .protobuf:
         do {
-            response.protobufPayload = try testMessage.serializedData()
+            response.protobufPayload = try testMessage.serializedBytes()
         } catch let e {
             response.serializeError = "Failed to serialize: \(e)"
         }
@@ -170,9 +168,9 @@ func buildResponse(serializedData: Data) -> Conformance_ConformanceResponse {
 
 func singleTest() throws -> Bool {
    if let indata = readRequest() {
-       let response = buildResponse(serializedData: indata)
-       let outdata = try response.serializedData()
-       writeResponse(data: outdata)
+       let response = buildResponse(serializedBytes: indata)
+       let outdata: [UInt8] = try response.serializedBytes()
+       writeResponse(bytes: outdata)
        return true
    } else {
       return false

Sources/SwiftProtobufCore/AnyMessageStorage.swift

@@ -43,13 +43,13 @@ fileprivate func emitVerboseTextForm(visitor: inout TextFormatEncodingVisitor, m
   visitor.visitAnyVerbose(value: message, typeURL: url)
 }
 
-fileprivate func asJSONObject(body: [UInt8]) -> [UInt8] {
+fileprivate func asJSONObject(body: [UInt8]) -> Data {
   let asciiOpenCurlyBracket = UInt8(ascii: "{")
   let asciiCloseCurlyBracket = UInt8(ascii: "}")
   var result = [asciiOpenCurlyBracket]
   result.append(contentsOf: body)
   result.append(asciiCloseCurlyBracket)
-  return result
+  return Data(result)
 }
 
 fileprivate func unpack(contentJSON: [UInt8],
@@ -102,7 +102,7 @@ internal class AnyMessageStorage {
         return Data(value)
       case .message(let message):
         do {
-          return try message.serializedData(partial: true)
+          return try message.serializedBytes(partial: true)
         } catch {
           return Data()
         }
@@ -115,14 +115,14 @@ internal class AnyMessageStorage {
                              extensions: SimpleExtensionMap(),
                              options: options,
                              as: messageType)
-          return try m.serializedData(partial: true)
+          return try m.serializedBytes(partial: true)
         } catch {
           return Data()
         }
       }
     }
     set {
-      state = .binary(Array(newValue))
+      state = .binary(newValue)
     }
   }
 
@@ -133,13 +133,13 @@ internal class AnyMessageStorage {
     // blob, i.e. - when decoding from binary, the spec doesn't include decoding
     // the binary blob, it is pass through. Instead there is a public api for
     // unpacking that takes new options when a developer decides to decode it.
-    case binary([UInt8])
+    case binary(Data)
     // a message
     case message(Message)
     // parsed JSON with the @type removed and the decoding options.
     case contentJSON([UInt8], JSONDecodingOptions)
   }
-  var state: InternalState = .binary([])
+  var state: InternalState = .binary(Data())
 
   static let defaultInstance = AnyMessageStorage()
 
@@ -179,8 +179,8 @@ internal class AnyMessageStorage {
         target = message
       } else {
         // Different type, serialize and parse.
-        let data = try msg.serializedData(partial: true)
-        target = try M(serializedBytes: Array(data), extensions: extensions, partial: true)
+        let bytes: [UInt8] = try msg.serializedBytes(partial: true)
+        target = try M(serializedBytes: bytes, extensions: extensions, partial: true)
       }
 
     case .contentJSON(let contentJSON, let options):
@@ -275,7 +275,7 @@ extension AnyMessageStorage {
         try! visitor.visitSingularStringField(value: _typeURL, fieldNumber: 1)
       }
       if !valueData.isEmpty {
-        try! visitor.visitSingularBytesField(value: Data(valueData), fieldNumber: 2)
+        try! visitor.visitSingularBytesField(value: valueData, fieldNumber: 2)
       }
 
     case .message(let msg):
@@ -300,7 +300,7 @@ extension AnyMessageStorage {
       }
       // Build a readable form of the JSON:
       let contentJSONAsObject = asJSONObject(body: contentJSON)
-      visitor.visitAnyJSONDataField(value: contentJSONAsObject)
+      visitor.visitAnyJSONBytesField(value: contentJSONAsObject)
     }
   }
 }
@@ -434,7 +434,7 @@ extension AnyMessageStorage {
         jsonEncoder.append(staticText: ",")
         // NOTE: This doesn't really take `options` into account since it is
         // just reflecting out what was taken in originally.
-        jsonEncoder.append(utf8Data: contentJSON)
+        jsonEncoder.append(utf8Bytes: contentJSON)
       }
       jsonEncoder.endObject()
       return jsonEncoder.stringResult
@@ -451,7 +451,7 @@ extension AnyMessageStorage {
     try decoder.scanner.skipRequiredObjectStart()
     // Reset state
     _typeURL = String()
-    state = .binary([])
+    state = .binary(Data())
     if decoder.scanner.skipOptionalObjectEnd() {
       return
     }

Sources/SwiftProtobufCore/BinaryDecoder.swift

@@ -787,6 +787,16 @@ internal struct BinaryDecoder: Decoder {
         }
     }
 
+    internal mutating func decodeSingularBytesField(value: inout [UInt8]) throws {
+        guard fieldWireFormat == WireFormat.lengthDelimited else {
+            return
+        }
+        var n: Int = 0
+        let p = try getFieldBodyBytes(count: &n)
+        value = Array(UnsafeRawBufferPointer(start: p, count: n))
+        consumed = true
+    }
+
     internal mutating func decodeSingularBytesField(value: inout Data) throws {
         guard fieldWireFormat == WireFormat.lengthDelimited else {
             return

Sources/SwiftProtobufCore/BinaryDelimited.swift

@@ -34,7 +34,7 @@ public enum BinaryDelimited {
     case tooLarge
   }
 
-  /// Serialize a single size-delimited message from the given stream. Delimited
+  /// Serialize a single size-delimited message to the given stream. Delimited
   /// format allows a single file or stream to contain multiple messages,
   /// whereas normally writing multiple non-delimited messages to the same
   /// stream would cause them to be merged. A delimited message is a varint
@@ -57,18 +57,18 @@ public enum BinaryDelimited {
     partial: Bool = false
   ) throws {
     // TODO: Revisit to avoid the extra buffering when encoding is streamed in general.
-    let serialized = try message.serializedData(partial: partial)
+    let serialized: [UInt8] = try message.serializedBytes(partial: partial)
     let totalSize = Varint.encodedSize(of: UInt64(serialized.count)) + serialized.count
-    var data = Data(count: totalSize)
-    data.withUnsafeMutableBytes { (body: UnsafeMutableRawBufferPointer) in
+    var bytes: [UInt8] = Array(repeating: 0, count: totalSize)
+    bytes.withUnsafeMutableBytes { (body: UnsafeMutableRawBufferPointer) in
       if let baseAddress = body.baseAddress, body.count > 0 {
         var encoder = BinaryEncoder(forWritingInto: baseAddress)
         encoder.putBytesValue(value: serialized)
       }
     }
 
     var written: Int = 0
-    data.withUnsafeBytes { (body: UnsafeRawBufferPointer) in
+    bytes.withUnsafeBytes { (body: UnsafeRawBufferPointer) in
       if let baseAddress = body.baseAddress, body.count > 0 {
         // This assumingMemoryBound is technically unsafe, but without SR-11078
         // (https://bugs.swift.org/browse/SR-11087) we don't have another option.

Sources/SwiftProtobufCore/BinaryEncoder.swift

@@ -28,8 +28,8 @@ internal struct BinaryEncoder {
         pointer = pointer.advanced(by: 1)
     }
 
-    private mutating func append(contentsOf data: Data) {
-        data.withUnsafeBytes { dataPointer in
+    private mutating func append<Bytes: SwiftProtobufContiguousBytes>(contentsOf bytes: Bytes) {
+        bytes.withUnsafeBytes { dataPointer in
             if let baseAddress = dataPointer.baseAddress, dataPointer.count > 0 {
                 pointer.copyMemory(from: baseAddress, byteCount: dataPointer.count)
                 pointer = pointer.advanced(by: dataPointer.count)
@@ -41,7 +41,7 @@ internal struct BinaryEncoder {
     private mutating func append(contentsOf bufferPointer: UnsafeRawBufferPointer) -> Int {
         let count = bufferPointer.count
         if let baseAddress = bufferPointer.baseAddress, count > 0 {
-            memcpy(pointer, baseAddress, count)
+            pointer.copyMemory(from: baseAddress, byteCount: count)
         }
         pointer = pointer.advanced(by: count)
         return count
@@ -92,34 +92,28 @@ internal struct BinaryEncoder {
     mutating func putFixedUInt64(value: UInt64) {
         var v = value.littleEndian
         let n = MemoryLayout<UInt64>.size
-        memcpy(pointer, &v, n)
+        pointer.copyMemory(from: &v, byteCount: n)
         pointer = pointer.advanced(by: n)
     }
 
     mutating func putFixedUInt32(value: UInt32) {
         var v = value.littleEndian
         let n = MemoryLayout<UInt32>.size
-        memcpy(pointer, &v, n)
+        pointer.copyMemory(from: &v, byteCount: n)
         pointer = pointer.advanced(by: n)
     }
 
     mutating func putFloatValue(value: Float) {
         let n = MemoryLayout<Float>.size
-        var v = value
-        var nativeBytes: UInt32 = 0
-        memcpy(&nativeBytes, &v, n)
-        var littleEndianBytes = nativeBytes.littleEndian
-        memcpy(pointer, &littleEndianBytes, n)
+        var v = value.bitPattern.littleEndian
+        pointer.copyMemory(from: &v, byteCount: n)
         pointer = pointer.advanced(by: n)
     }
 
     mutating func putDoubleValue(value: Double) {
         let n = MemoryLayout<Double>.size
-        var v = value
-        var nativeBytes: UInt64 = 0
-        memcpy(&nativeBytes, &v, n)
-        var littleEndianBytes = nativeBytes.littleEndian
-        memcpy(pointer, &littleEndianBytes, n)
+        var v = value.bitPattern.littleEndian
+        pointer.copyMemory(from: &v, byteCount: n)
         pointer = pointer.advanced(by: n)
     }
 
@@ -142,7 +136,7 @@ internal struct BinaryEncoder {
         }
     }
 
-    mutating func putBytesValue(value: Data) {
+    mutating func putBytesValue<Bytes: SwiftProtobufContiguousBytes>(value: Bytes) {
         putVarInt(value: value.count)
         append(contentsOf: value)
     }

Sources/SwiftProtobufCore/Decoder.swift

@@ -154,6 +154,8 @@ public protocol Decoder {
   mutating func decodeRepeatedStringField(value: inout [String]) throws
   /// Decode a bytes value to non-`Optional` field storage
   mutating func decodeSingularBytesField(value: inout Data) throws
+  /// Decode a bytes value to non-`Optional` field storage
+  mutating func decodeSingularBytesField(value: inout [UInt8]) throws
   /// Decode a bytes value to `Optional` field storage
   mutating func decodeSingularBytesField(value: inout Data?) throws
   /// Decode bytes values to repeated field storage