From e93936e00d0ca3f594e54877deca90afe21f8083 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 29 Nov 2023 15:10:08 -0800 Subject: [PATCH 1/2] Binary encode from back-to-front Our current front-to-back binary encoder has a subtle performance problem: In order to allocate the correct number of bytes for the size of a sub-message, it needs to know the final size of the sub-message. This results in recursive calls to size each message, leading to overall performance that is quadratic in the depth of nesting. This is not usually a serious problem since few people have messages with more than 2 or 3 layers of nesting. This PR changes the encoder to encode from the end of the buffer back towards the front. This allows us to write the size after we finish each sub-message, avoiding the recursive sizing calls. With this version, we have only one sizing traversal in the initial top-level encoding request in order to properly allocate the output buffer. Working from back to front does mean that individual fields get written in the opposite order. This is _mostly_ not a problem: Protobuf explicitly states that decoders must accept fields in any order. The one exception is for repeated fields, which we handle here by iterating the arrays backwards inside the encoder. A few cases where order might matter: * Unrecognized enum cases in repeated fields are treted as "unknown" fields which means that re-serializing puts them into a different place. Since this code puts the unknown fields at the beginning of the buffer rather than the end, this means that we've changed the resulting order after deserializing/reserializing. * The conformance test has one test case that verifies merging behavior and seems very sensitive to the order of fields. I suspect this is a bug in the test, but need to check further. --- .../SwiftProtobuf/BinaryReverseEncoder.swift | 156 ++++++ .../BinaryReverseEncodingVisitor.swift | 497 ++++++++++++++++++ Sources/SwiftProtobuf/CMakeLists.txt | 2 + .../Message+BinaryAdditions.swift | 2 +- 4 files changed, 656 insertions(+), 1 deletion(-) create mode 100644 Sources/SwiftProtobuf/BinaryReverseEncoder.swift create mode 100644 Sources/SwiftProtobuf/BinaryReverseEncodingVisitor.swift diff --git a/Sources/SwiftProtobuf/BinaryReverseEncoder.swift b/Sources/SwiftProtobuf/BinaryReverseEncoder.swift new file mode 100644 index 000000000..9700864a5 --- /dev/null +++ b/Sources/SwiftProtobuf/BinaryReverseEncoder.swift @@ -0,0 +1,156 @@ +// Sources/SwiftProtobuf/BinaryReverseEncoder.swift - Binary encoding support +// +// Copyright (c) 2014 - 2016 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/main/LICENSE.txt +// +// ----------------------------------------------------------------------------- +/// +/// Core support for protobuf binary encoding. Note that this is built +/// on the general traversal machinery. +/// +// ----------------------------------------------------------------------------- + +import Foundation + +/// Encoder for Binary Protocol Buffer format +internal struct BinaryReverseEncoder { + private var pointer: UnsafeMutableRawPointer + private var buffer: UnsafeMutableRawBufferPointer + + init(forWritingInto buffer: UnsafeMutableRawBufferPointer) { + self.buffer = buffer + self.pointer = buffer.baseAddress! + buffer.count + } + + private mutating func prepend(_ byte: UInt8) { + consume(1) + pointer.storeBytes(of: byte, as: UInt8.self) + } + + private mutating func prepend(contentsOf bytes: Bytes) { + bytes.withUnsafeBytes { dataPointer in + if let baseAddress = dataPointer.baseAddress, dataPointer.count > 0 { + consume(dataPointer.count) + pointer.copyMemory(from: baseAddress, byteCount: dataPointer.count) + } + } + } + + internal var used: Int { + return pointer.distance(to: buffer.baseAddress!) + buffer.count + } + + internal var remainder: UnsafeMutableRawBufferPointer { + return UnsafeMutableRawBufferPointer(start: buffer.baseAddress!, + count: buffer.count - used) + } + + internal mutating func consume(_ bytes: Int) { + pointer = pointer.advanced(by: -bytes) + } + + @discardableResult + private mutating func prepend(contentsOf bufferPointer: UnsafeRawBufferPointer) -> Int { + let count = bufferPointer.count + consume(count) + if let baseAddress = bufferPointer.baseAddress, count > 0 { + pointer.copyMemory(from: baseAddress, byteCount: count) + } + return count + } + + mutating func appendUnknown(data: Data) { + prepend(contentsOf: data) + } + + mutating func startField(fieldNumber: Int, wireFormat: WireFormat) { + startField(tag: FieldTag(fieldNumber: fieldNumber, wireFormat: wireFormat)) + } + + mutating func startField(tag: FieldTag) { + putVarInt(value: UInt64(tag.rawValue)) + } + + mutating func putVarInt(value: UInt64) { + if value > 127 { + putVarInt(value: value >> 7) + prepend(UInt8(value & 0x7f | 0x80)) + } else { + prepend(UInt8(value)) + } + } + + mutating func putVarInt(value: Int64) { + putVarInt(value: UInt64(bitPattern: value)) + } + + mutating func putVarInt(value: Int) { + putVarInt(value: Int64(value)) + } + + mutating func putZigZagVarInt(value: Int64) { + let coded = ZigZag.encoded(value) + putVarInt(value: coded) + } + + mutating func putBoolValue(value: Bool) { + prepend(value ? 1 : 0) + } + + mutating func putFixedUInt64(value: UInt64) { + var v = value.littleEndian + let n = MemoryLayout.size + consume(n) + pointer.copyMemory(from: &v, byteCount: n) + } + + mutating func putFixedUInt32(value: UInt32) { + var v = value.littleEndian + let n = MemoryLayout.size + consume(n) + pointer.copyMemory(from: &v, byteCount: n) + } + + mutating func putFloatValue(value: Float) { + let n = MemoryLayout.size + var v = value.bitPattern.littleEndian + consume(n) + pointer.copyMemory(from: &v, byteCount: n) + } + + mutating func putDoubleValue(value: Double) { + let n = MemoryLayout.size + var v = value.bitPattern.littleEndian + consume(n) + pointer.copyMemory(from: &v, byteCount: n) + } + + // Write a string field, including the leading index/tag value. + mutating func putStringValue(value: String) { + let utf8 = value.utf8 + // If the String does not support an internal representation in a form + // of contiguous storage, body is not called and nil is returned. + let isAvailable = utf8.withContiguousStorageIfAvailable { (body: UnsafeBufferPointer) -> Int in + let r = prepend(contentsOf: UnsafeRawBufferPointer(body)) + putVarInt(value: body.count) + return r + } + if isAvailable == nil { + precondition(false) + let count = utf8.count + putVarInt(value: count) + for b in utf8 { + pointer.storeBytes(of: b, as: UInt8.self) + consume(1) + } + } + } + + mutating func putBytesValue(value: Bytes) { + prepend(contentsOf: value) + putVarInt(value: value.count) + } +} diff --git a/Sources/SwiftProtobuf/BinaryReverseEncodingVisitor.swift b/Sources/SwiftProtobuf/BinaryReverseEncodingVisitor.swift new file mode 100644 index 000000000..fc75fd7c2 --- /dev/null +++ b/Sources/SwiftProtobuf/BinaryReverseEncodingVisitor.swift @@ -0,0 +1,497 @@ +// Sources/SwiftProtobuf/BinaryReverseEncodingVisitor.swift - Binary encoding support +// +// Copyright (c) 2014 - 2016 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/main/LICENSE.txt +// +// ----------------------------------------------------------------------------- +/// +/// Core support for protobuf binary encoding. Note that this is built +/// on the general traversal machinery. +/// +// ----------------------------------------------------------------------------- + +import Foundation + +/// Visitor that encodes a message graph in the protobuf binary wire format. +internal struct BinaryReverseEncodingVisitor: Visitor { + private let options: BinaryEncodingOptions + + var encoder: BinaryReverseEncoder + + /// Creates a new visitor that writes the binary-coded message into the memory + /// at the given pointer. + /// + /// - Precondition: `pointer` must point to an allocated block of memory that + /// is large enough to hold the entire encoded message. For performance + /// reasons, the encoder does not make any attempts to verify this. + init(forWritingInto buffer: UnsafeMutableRawBufferPointer, options: BinaryEncodingOptions) { + self.encoder = BinaryReverseEncoder(forWritingInto: buffer) + self.options = options + } + + mutating func visitUnknown(bytes: Data) throws { + encoder.appendUnknown(data: bytes) + } + + mutating func visitSingularFloatField(value: Float, fieldNumber: Int) throws { + encoder.putFloatValue(value: value) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .fixed32) + } + + mutating func visitSingularDoubleField(value: Double, fieldNumber: Int) throws { + encoder.putDoubleValue(value: value) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .fixed64) + } + + mutating func visitSingularInt64Field(value: Int64, fieldNumber: Int) throws { + try visitSingularUInt64Field(value: UInt64(bitPattern: value), fieldNumber: fieldNumber) + } + + mutating func visitSingularUInt64Field(value: UInt64, fieldNumber: Int) throws { + encoder.putVarInt(value: value) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .varint) + } + + mutating func visitSingularSInt32Field(value: Int32, fieldNumber: Int) throws { + try visitSingularSInt64Field(value: Int64(value), fieldNumber: fieldNumber) + } + + mutating func visitSingularSInt64Field(value: Int64, fieldNumber: Int) throws { + try visitSingularUInt64Field(value: ZigZag.encoded(value), fieldNumber: fieldNumber) + } + + mutating func visitSingularFixed32Field(value: UInt32, fieldNumber: Int) throws { + encoder.putFixedUInt32(value: value) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .fixed32) + } + + mutating func visitSingularFixed64Field(value: UInt64, fieldNumber: Int) throws { + encoder.putFixedUInt64(value: value) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .fixed64) + } + + mutating func visitSingularSFixed32Field(value: Int32, fieldNumber: Int) throws { + try visitSingularFixed32Field(value: UInt32(bitPattern: value), fieldNumber: fieldNumber) + } + + mutating func visitSingularSFixed64Field(value: Int64, fieldNumber: Int) throws { + try visitSingularFixed64Field(value: UInt64(bitPattern: value), fieldNumber: fieldNumber) + } + + mutating func visitSingularBoolField(value: Bool, fieldNumber: Int) throws { + try visitSingularUInt64Field(value: value ? 1 : 0, fieldNumber: fieldNumber) + } + + mutating func visitSingularStringField(value: String, fieldNumber: Int) throws { + encoder.putStringValue(value: value) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitSingularBytesField(value: Data, fieldNumber: Int) throws { + encoder.putBytesValue(value: value) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitSingularEnumField(value: E, + fieldNumber: Int) throws { + try visitSingularUInt64Field(value: UInt64(bitPattern: Int64(value.rawValue)), + fieldNumber: fieldNumber) + } + + mutating func visitSingularMessageField(value: M, + fieldNumber: Int) throws { + let before = encoder.used + try value.traverse(visitor: &self) + let length = encoder.used - before + encoder.putVarInt(value: length) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitSingularGroupField(value: G, fieldNumber: Int) throws { + encoder.startField(fieldNumber: fieldNumber, wireFormat: .endGroup) + try value.traverse(visitor: &self) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .startGroup) + } + + // Repeated Fields + + public mutating func visitRepeatedFloatField(value: [Float], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularFloatField(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedDoubleField(value: [Double], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularDoubleField(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedInt32Field(value: [Int32], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularInt32Field(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedInt64Field(value: [Int64], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularInt64Field(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedUInt32Field(value: [UInt32], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularUInt32Field(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedUInt64Field(value: [UInt64], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularUInt64Field(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedSInt32Field(value: [Int32], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularSInt32Field(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedSInt64Field(value: [Int64], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularSInt64Field(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedFixed32Field(value: [UInt32], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularFixed32Field(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedFixed64Field(value: [UInt64], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularFixed64Field(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedSFixed32Field(value: [Int32], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularSFixed32Field(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedSFixed64Field(value: [Int64], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularSFixed64Field(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedBoolField(value: [Bool], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularBoolField(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedStringField(value: [String], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularStringField(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedBytesField(value: [Data], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularBytesField(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedEnumField(value: [E], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularEnumField(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedMessageField(value: [M], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularMessageField(value: v, fieldNumber: fieldNumber) + } + } + + public mutating func visitRepeatedGroupField(value: [G], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + try visitSingularGroupField(value: v, fieldNumber: fieldNumber) + } + } + + // Packed Fields + + mutating func visitPackedFloatField(value: [Float], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putFloatValue(value: v) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedDoubleField(value: [Double], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putDoubleValue(value: v) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedInt32Field(value: [Int32], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putVarInt(value: Int64(v)) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedInt64Field(value: [Int64], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putVarInt(value: v) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedSInt32Field(value: [Int32], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putZigZagVarInt(value: Int64(v)) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedSInt64Field(value: [Int64], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putZigZagVarInt(value: v) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedUInt32Field(value: [UInt32], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putVarInt(value: UInt64(v)) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedUInt64Field(value: [UInt64], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putVarInt(value: v) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedFixed32Field(value: [UInt32], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putFixedUInt32(value: v) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedFixed64Field(value: [UInt64], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putFixedUInt64(value: v) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedSFixed32Field(value: [Int32], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putFixedUInt32(value: UInt32(bitPattern: v)) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedSFixed64Field(value: [Int64], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putFixedUInt64(value: UInt64(bitPattern: v)) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedBoolField(value: [Bool], fieldNumber: Int) throws { + assert(!value.isEmpty) + for v in value.reversed() { + encoder.putVarInt(value: v ? 1 : 0) + } + encoder.putVarInt(value: value.count) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitPackedEnumField(value: [E], fieldNumber: Int) throws { + assert(!value.isEmpty) + let before = encoder.used + for v in value.reversed() { + encoder.putVarInt(value: v.rawValue) + } + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + + mutating func visitMapField( + fieldType: _ProtobufMap.Type, + value: _ProtobufMap.BaseType, + fieldNumber: Int + ) throws { + try iterateAndEncode( + map: value, fieldNumber: fieldNumber, isOrderedBefore: KeyType._lessThan, + encodeWithVisitor: { visitor, key, value in + try ValueType.visitSingular(value: value, fieldNumber: 2, with: &visitor) + try KeyType.visitSingular(value: key, fieldNumber: 1, with: &visitor) + } + ) + } + + mutating func visitMapField( + fieldType: _ProtobufEnumMap.Type, + value: _ProtobufEnumMap.BaseType, + fieldNumber: Int + ) throws where ValueType.RawValue == Int { + try iterateAndEncode( + map: value, fieldNumber: fieldNumber, isOrderedBefore: KeyType._lessThan, + encodeWithVisitor: { visitor, key, value in + try visitor.visitSingularEnumField(value: value, fieldNumber: 2) + try KeyType.visitSingular(value: key, fieldNumber: 1, with: &visitor) + } + ) + } + + mutating func visitMapField( + fieldType: _ProtobufMessageMap.Type, + value: _ProtobufMessageMap.BaseType, + fieldNumber: Int + ) throws { + try iterateAndEncode( + map: value, fieldNumber: fieldNumber, isOrderedBefore: KeyType._lessThan, + encodeWithVisitor: { visitor, key, value in + try visitor.visitSingularMessageField(value: value, fieldNumber: 2) + try KeyType.visitSingular(value: key, fieldNumber: 1, with: &visitor) + } + ) + } + + /// Helper to encapsulate the common structure of iterating over a map + /// and encoding the keys and values. + private mutating func iterateAndEncode( + map: Dictionary, + fieldNumber: Int, + isOrderedBefore: (K, K) -> Bool, + encodeWithVisitor: (inout BinaryReverseEncodingVisitor, K, V) throws -> () + ) throws { + if options.useDeterministicOrdering { + for (k,v) in map.sorted(by: { isOrderedBefore( $0.0, $1.0) }).reversed() { + let before = encoder.used + try encodeWithVisitor(&self, k, v) + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + } else { + for (k,v) in map { + let before = encoder.used + try encodeWithVisitor(&self, k, v) + encoder.putVarInt(value: encoder.used - before) + encoder.startField(fieldNumber: fieldNumber, wireFormat: .lengthDelimited) + } + } + } + + mutating func visitExtensionFieldsAsMessageSet( + fields: ExtensionFieldValueSet, + start: Int, + end: Int + ) throws { + var subVisitor = BinaryReverseEncodingMessageSetVisitor(encoder: encoder, options: options) + try fields.traverse(visitor: &subVisitor, start: start, end: end) + encoder = subVisitor.encoder + } +} + +extension BinaryReverseEncodingVisitor { + + // Helper Visitor to when writing out the extensions as MessageSets. + internal struct BinaryReverseEncodingMessageSetVisitor: SelectiveVisitor { + private let options: BinaryEncodingOptions + + var encoder: BinaryReverseEncoder + + init(encoder: BinaryReverseEncoder, options: BinaryEncodingOptions) { + self.options = options + self.encoder = encoder + } + + mutating func visitSingularMessageField(value: M, fieldNumber: Int) throws { + encoder.putVarInt(value: Int64(WireFormat.MessageSet.Tags.itemEnd.rawValue)) + + var subVisitor = BinaryReverseEncodingVisitor( + forWritingInto: encoder.remainder, options: options + ) + try value.traverse(visitor: &subVisitor) + encoder.consume(subVisitor.encoder.used) + encoder.putVarInt(value: subVisitor.encoder.used) + + encoder.putVarInt(value: Int64(WireFormat.MessageSet.Tags.message.rawValue)) + encoder.putVarInt(value: fieldNumber) + encoder.putVarInt(value: Int64(WireFormat.MessageSet.Tags.typeId.rawValue)) + encoder.putVarInt(value: Int64(WireFormat.MessageSet.Tags.itemStart.rawValue)) + } + + // SelectiveVisitor handles the rest. + } +} diff --git a/Sources/SwiftProtobuf/CMakeLists.txt b/Sources/SwiftProtobuf/CMakeLists.txt index 74816c320..3913e6286 100644 --- a/Sources/SwiftProtobuf/CMakeLists.txt +++ b/Sources/SwiftProtobuf/CMakeLists.txt @@ -11,6 +11,8 @@ add_library(SwiftProtobuf BinaryEncodingError.swift BinaryEncodingSizeVisitor.swift BinaryEncodingVisitor.swift + BinaryReverseEncoder.swift + BinaryReverseEncodingVisitor.swift CustomJSONCodable.swift Data+Extensions.swift Decoder.swift diff --git a/Sources/SwiftProtobuf/Message+BinaryAdditions.swift b/Sources/SwiftProtobuf/Message+BinaryAdditions.swift index 2580ed450..257ad51d1 100644 --- a/Sources/SwiftProtobuf/Message+BinaryAdditions.swift +++ b/Sources/SwiftProtobuf/Message+BinaryAdditions.swift @@ -53,7 +53,7 @@ extension Message { var data = Bytes(repeating: 0, count: requiredSize) try data.withUnsafeMutableBytes { (body: UnsafeMutableRawBufferPointer) in - var visitor = BinaryEncodingVisitor(forWritingInto: body, options: options) + var visitor = BinaryReverseEncodingVisitor(forWritingInto: body, options: options) try traverse(visitor: &visitor) // Currently not exposing this from the api because it really would be // an internal error in the library and should never happen. From e976607db17f20e884d813b9a3c232aacaf7828b Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Wed, 29 Nov 2023 17:46:45 -0800 Subject: [PATCH 2/2] Implement the missing non-contiguous String case While I'm digging around, fix some whitespace errors and simplify a few minor points in the code. --- .../SwiftProtobuf/BinaryReverseEncoder.swift | 44 ++++++++----------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/Sources/SwiftProtobuf/BinaryReverseEncoder.swift b/Sources/SwiftProtobuf/BinaryReverseEncoder.swift index 9700864a5..c2d1bffa9 100644 --- a/Sources/SwiftProtobuf/BinaryReverseEncoder.swift +++ b/Sources/SwiftProtobuf/BinaryReverseEncoder.swift @@ -31,12 +31,7 @@ internal struct BinaryReverseEncoder { } private mutating func prepend(contentsOf bytes: Bytes) { - bytes.withUnsafeBytes { dataPointer in - if let baseAddress = dataPointer.baseAddress, dataPointer.count > 0 { - consume(dataPointer.count) - pointer.copyMemory(from: baseAddress, byteCount: dataPointer.count) - } - } + bytes.withUnsafeBytes { prepend(contentsOf: $0) } } internal var used: Int { @@ -45,21 +40,19 @@ internal struct BinaryReverseEncoder { internal var remainder: UnsafeMutableRawBufferPointer { return UnsafeMutableRawBufferPointer(start: buffer.baseAddress!, - count: buffer.count - used) + count: buffer.count - used) } internal mutating func consume(_ bytes: Int) { pointer = pointer.advanced(by: -bytes) } - @discardableResult - private mutating func prepend(contentsOf bufferPointer: UnsafeRawBufferPointer) -> Int { + private mutating func prepend(contentsOf bufferPointer: UnsafeRawBufferPointer) { let count = bufferPointer.count - consume(count) + consume(count) if let baseAddress = bufferPointer.baseAddress, count > 0 { pointer.copyMemory(from: baseAddress, byteCount: count) } - return count } mutating func appendUnknown(data: Data) { @@ -76,11 +69,11 @@ internal struct BinaryReverseEncoder { mutating func putVarInt(value: UInt64) { if value > 127 { - putVarInt(value: value >> 7) + putVarInt(value: value >> 7) prepend(UInt8(value & 0x7f | 0x80)) } else { prepend(UInt8(value)) - } + } } mutating func putVarInt(value: Int64) { @@ -103,28 +96,28 @@ internal struct BinaryReverseEncoder { mutating func putFixedUInt64(value: UInt64) { var v = value.littleEndian let n = MemoryLayout.size - consume(n) + consume(n) pointer.copyMemory(from: &v, byteCount: n) } mutating func putFixedUInt32(value: UInt32) { var v = value.littleEndian let n = MemoryLayout.size - consume(n) + consume(n) pointer.copyMemory(from: &v, byteCount: n) } mutating func putFloatValue(value: Float) { let n = MemoryLayout.size var v = value.bitPattern.littleEndian - consume(n) + consume(n) pointer.copyMemory(from: &v, byteCount: n) } mutating func putDoubleValue(value: Double) { let n = MemoryLayout.size var v = value.bitPattern.littleEndian - consume(n) + consume(n) pointer.copyMemory(from: &v, byteCount: n) } @@ -133,19 +126,20 @@ internal struct BinaryReverseEncoder { let utf8 = value.utf8 // If the String does not support an internal representation in a form // of contiguous storage, body is not called and nil is returned. - let isAvailable = utf8.withContiguousStorageIfAvailable { (body: UnsafeBufferPointer) -> Int in - let r = prepend(contentsOf: UnsafeRawBufferPointer(body)) + let usedContiguousCopy = utf8.withContiguousStorageIfAvailable { (body: UnsafeBufferPointer) -> Bool in + prepend(contentsOf: UnsafeRawBufferPointer(body)) putVarInt(value: body.count) - return r + return true } - if isAvailable == nil { - precondition(false) + if usedContiguousCopy == nil { let count = utf8.count - putVarInt(value: count) + consume(count) + var i = 0 for b in utf8 { - pointer.storeBytes(of: b, as: UInt8.self) - consume(1) + pointer[i] = b + i += 1 } + putVarInt(value: count) } }