From 2f3536a1b2cd18f2ac9e18111ba1ccfddd16c8c2 Mon Sep 17 00:00:00 2001 From: Rick Newton-Rogers Date: Tue, 29 Sep 2026 18:03:45 -0400 Subject: [PATCH] Keep retained capacity in `FrameArray.add(frames:)` A queue drained through `drainArrayKeepingCapacity` holds an empty buffer on purpose. Adopting the incoming storage whenever this side is empty threw that buffer away and took a smaller one, so the next batch reallocated. Adopt the incoming storage only when this side would otherwise have to grow. Measured on my Mac against `main` with the package's QUIC benchmark tools. Allocation counts come from full malloc stack logging, which records every allocation: QUICTransfer -size 1200, per message 24.37 -> 23.38 QUICTransfer, per 500 KB transfer 6,187.6 -> 6,177.1 QUICHandshake, per connection 1,947.4 -> 1,944.9 QUICStreamLoad, per stream 111.8 -> 112.2 The saving depends on timing: it only applies when the queue has drained by the time the next batch arrives, which happened about once per message here. Wall-clock time comes from running `main`, this change and the other changes measured alongside it in a rotating order for 9 rounds, and comparing each run with `main`'s in the same round. Changes moved paths they do not touch by up to about 1.3%, so differences that size count as noise. Nothing changed beyond noise. --- .../SwiftNetwork/Protocols/FrameArray.swift | 10 +- .../SwiftNetworkFrameArrayCapacityTests.swift | 93 +++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 Tests/SwiftNetworkTests/SwiftNetworkFrameArrayCapacityTests.swift diff --git a/Sources/SwiftNetwork/Protocols/FrameArray.swift b/Sources/SwiftNetwork/Protocols/FrameArray.swift index 33764e57..44d8dc75 100644 --- a/Sources/SwiftNetwork/Protocols/FrameArray.swift +++ b/Sources/SwiftNetwork/Protocols/FrameArray.swift @@ -48,7 +48,10 @@ public struct FrameArray: ~Copyable { } public mutating func add(frames: consuming FrameArray) { - if self.frames.isEmpty { + // Taking the incoming storage is only a win when this side would have to grow to hold + // the incoming frames. A drained queue that kept its capacity has room already, and + // adopting a smaller buffer would throw that capacity away. + if self.frames.isEmpty, self.frames.capacity < frames.count { self.frames = frames.frames } else { while let first = frames.frames.popFirst() { @@ -65,6 +68,11 @@ public struct FrameArray: ~Copyable { frames.count } + /// The number of frames the array can hold before its storage has to grow. + var capacity: Int { + frames.capacity + } + #if !NETWORK_EMBEDDED @_lifetime(borrow self) func bytes(at index: Int) -> RawSpan? { diff --git a/Tests/SwiftNetworkTests/SwiftNetworkFrameArrayCapacityTests.swift b/Tests/SwiftNetworkTests/SwiftNetworkFrameArrayCapacityTests.swift new file mode 100644 index 00000000..dc005d7d --- /dev/null +++ b/Tests/SwiftNetworkTests/SwiftNetworkFrameArrayCapacityTests.swift @@ -0,0 +1,93 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2026 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import Testing + +#if canImport(SwiftNetwork) +@_spi(Essentials) @_spi(ProtocolProvider) @testable import SwiftNetwork +#endif + +// Swift Testing rejects `@available` on a suite or a test function, so each test narrows +// availability in its own body instead. +@Suite("FrameArray capacity") +struct SwiftNetworkFrameArrayCapacityTests { + /// A send queue drained through `drainArrayKeepingCapacity` must keep the storage it was + /// left with; if `add(frames:)` adopts the incoming buffer instead, every batch reallocates. + @Test("A drained array keeps its capacity when frames are added") + func drainedArrayKeepsCapacityWhenAddingFrames() { + guard #available(Network 0.1.0, *) else { return } + + var queue = FrameArray() + for _ in 0..<8 { + queue.add(frame: Frame(count: 16)) + } + var drained = queue.drainArrayKeepingCapacity() + drained.finalizeAllFramesAsFailed() + + let retainedCapacity = queue.capacity + #expect(retainedCapacity >= 8) + + var incoming = FrameArray(capacity: 1) + incoming.add(frame: Frame(count: 16)) + queue.add(frames: incoming) + + #expect(queue.capacity == retainedCapacity) + #expect(queue.count == 1) + queue.finalizeAllFramesAsFailed() + } + + /// An array with no room must still take the incoming storage rather than grow its own, + /// which is what keeps the first hand-off of a batch free of an allocation. + @Test("An empty array with no capacity adopts the incoming storage") + func emptyArrayWithoutCapacityAdoptsIncomingStorage() { + guard #available(Network 0.1.0, *) else { return } + + var queue = FrameArray() + #expect(queue.capacity == 0) + + var incoming = FrameArray(capacity: 8) + for _ in 0..<8 { + incoming.add(frame: Frame(count: 16)) + } + let incomingCapacity = incoming.capacity + queue.add(frames: incoming) + + #expect(queue.capacity == incomingCapacity) + #expect(queue.count == 8) + queue.finalizeAllFramesAsFailed() + } + + /// Frames already held must stay ahead of the ones being added, whichever buffer survives. + @Test("Adding frames preserves order") + func addingFramesPreservesOrder() { + guard #available(Network 0.1.0, *) else { return } + + var queue = FrameArray() + queue.add(frame: Frame(count: 10)) + queue.add(frame: Frame(count: 20)) + + var incoming = FrameArray() + incoming.add(frame: Frame(count: 30)) + incoming.add(frame: Frame(count: 40)) + queue.add(frames: incoming) + + var lengths: [Int] = [] + queue.iterateImmutableFrames { frame in + lengths.append(frame.unclaimedLength) + return true + } + #expect(lengths == [10, 20, 30, 40]) + queue.finalizeAllFramesAsFailed() + } +}