Skip to content

Make hashing cheaper on hot paths #85

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
glbrntt wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Make hashing cheaper on hot paths #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b55763
Make hashing cheaper for transport parameters and stream ID
glbrntt Aug 14, 2026
ac64545
Implement _rawHashValue(seed:) for MultiplexedFlowIdentifier
glbrntt Aug 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions Sources/SwiftNetwork/Protocols/ManyToManyProtocol.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,16 +166,25 @@ where Path.LowerProtocol == OutboundDatagramLinkage {
}

public func hash(into hasher: inout Hasher) {
hasher.combine(self.rawHashKey)
}

private var rawHashKey: UInt64 {
// Use the bottom two bits as a discriminator for the different cases.
switch self {
case .allFlows:
hasher.combine(0)
return 0
case .outboundFlow(let index):
hasher.combine(index)
return UInt64(bitPattern: Int64(index)) << 2 | 0b01
case .inboundFlow(let index):
hasher.combine(index)
return UInt64(bitPattern: Int64(index)) << 2 | 0b10
}
}

public func _rawHashValue(seed: Int) -> Int {
self.rawHashKey._rawHashValue(seed: seed)
}
Comment on lines +184 to +186

@glbrntt glbrntt Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to get @lorentey's opinion on the use of this particular API since it's underscored...

@lorentey lorentey Aug 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great use of _rawHashValue! Forwarding to UInt64's implementation still ensures strong hashing, and marking the case in the two lower bits helps reduce collisions in the original implementation. 👍


public var debugDescription: String {
switch self {
case .allFlows: return "All Flows"
Expand Down
27 changes: 15 additions & 12 deletions Sources/SwiftNetwork/QUIC/QUICConnection.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,10 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol,
// List of streams that have app input data in their reassembly queue.
var pendingReassemblyDequeue = QUICStreamList.pendingReassemblyDequeueList()

private(set) var knownFlows = [QUICStreamID: MultiplexedFlowIdentifier]()
// The logical key choice is 'QUICStreamID', however, Swift special cases the hashing of

@tfpauly tfpauly Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glbrntt what if we just made QUICStreamID be RawRepresentable and just be a raw UInt64? That's all it stores anyway.

@glbrntt glbrntt Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into that: sadly RawRepresentable hits the slow path! The default implementation of _rawHashValue(seed:) is this:

public func _rawHashValue(seed: Int) -> Int {
  var hasher = Hasher(_seed: seed)
  hasher.combine(self)
  return hasher._finalize()
}

So for RawRepresentable to get the fast path of not creating the Hasher it would have to forgo allowing combine(_:) to be customized.

@rpaulo rpaulo Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to consider typealias QUICStreamID = UInt64 and then deal with the methods as extensions to UInt64 to avoid more problems like this?

// various primitives (by avoiding the construction of a Hasher altogether). The result is
// that hashing the raw value is significantly cheaper which adds up on hot paths.
private(set) var knownFlows = [UInt64: MultiplexedFlowIdentifier]()

@agnosticdev agnosticdev Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you for uncovering this @glbrntt !


private(set) var localCIDLength: Int = 0
private var initialSourceConnectionID: QUICConnectionID?
Expand Down Expand Up @@ -2446,7 +2449,7 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol,
if dataLength > 0 {
processOutbound(frame: frame, flowID: flowID, stream: stream, isLast: isFinal)
continue
} else if isFinal, let _ = knownFlows[streamID] {
} else if isFinal, let _ = knownFlows[streamID.value] {
log.datapath("Treating zero length fin as a stop message")
disconnect(flow: flowID, direction: .outbound)
} else {
Expand Down Expand Up @@ -2631,7 +2634,7 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol,

if let streamID {
log.debug("Set known flow \(flowID.debugDescription) for key \(streamID)")
knownFlows[streamID] = flowID
knownFlows[streamID.value] = flowID
if isUnidirectional {
self.unidirectionalStreams.incrementActiveStreams()
} else {
Expand Down Expand Up @@ -4323,7 +4326,7 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol,

func deliverInboundAbortedEvent(stream: QUICStreamInstance, error: NetworkError?) {
guard let streamID = stream.streamID,
let _ = knownFlows[streamID]
let _ = knownFlows[streamID.value]
else {
log.error("Cannot deliver inbound aborted event: no flow for stream \(stream.streamID?.value ?? 0)")
return
Expand All @@ -4333,7 +4336,7 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol,

func handleStreamClose(stream: QUICStreamInstance, error: NetworkError?) {
guard let streamID = stream.streamID,
let flowID = knownFlows[streamID]
let flowID = knownFlows[streamID.value]
else {
return
}
Expand All @@ -4347,7 +4350,7 @@ public final class QUICConnection: ManyToManyApplicationStreamProtocol,
}
stream.closed = true
deliverDisconnectedEvent(flow: flowID, error: error)
knownFlows.removeValue(forKey: streamID)
knownFlows.removeValue(forKey: streamID.value)
log.datapath("closed stream \(streamID.value)")

if let streamID = stream.streamID {
Expand Down Expand Up @@ -4559,7 +4562,7 @@ extension QUICConnection {
return false
}

let knownFlowID = knownFlows[streamID]
let knownFlowID = knownFlows[streamID.value]
if knownFlowID == nil {
let inboundStreamResult = createInboundStreams(streamID: streamID)
if frame.isFinal && inboundStreamResult.checkZombie {
Expand All @@ -4577,7 +4580,7 @@ extension QUICConnection {
return false
}
}
guard let flowID = knownFlowID ?? knownFlows[streamID] else {
guard let flowID = knownFlowID ?? knownFlows[streamID.value] else {
frame.frame.finalize(success: true)
return true
}
Expand Down Expand Up @@ -4678,7 +4681,7 @@ extension QUICConnection {
}

// 2. Lookup stream
let knownFlowID = knownFlows[streamID]
let knownFlowID = knownFlows[streamID.value]

// 3. If new stream
if knownFlowID == nil {
Expand Down Expand Up @@ -5218,7 +5221,7 @@ extension QUICConnection {

log.debug("Updating flow \(flowID.debugDescription) for key \(streamID)")

knownFlows[streamID] = flowID
knownFlows[streamID.value] = flowID
if !stream.unidirectional {
self.bidirectionalStreams.incrementActiveStreams()
stream.receiveState.change(logIDString: stream.logPrefix, to: .receive)
Expand Down Expand Up @@ -5425,7 +5428,7 @@ extension QUICConnection {
let newFlowIdentifier = newStream.identifier
multiplexedFlows[newFlowIdentifier] = newStream

knownFlows[newStreamID] = newFlowIdentifier
knownFlows[newStreamID.value] = newFlowIdentifier
newStream.setup(
streamID: newStreamID,
logPrefixer: logPrefixer
Expand Down Expand Up @@ -5755,7 +5758,7 @@ extension QUICConnection {
}

func streamFromStreamID(_ streamID: QUICStreamID) -> QUICStreamInstance? {
let knownFlowID = knownFlows[streamID]
let knownFlowID = knownFlows[streamID.value]
guard let flowID = knownFlowID else {
return nil
}
Expand Down
6 changes: 3 additions & 3 deletions Sources/SwiftNetwork/QUIC/QUICFrame.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ struct FrameResetStream: ~Copyable, QUICFrameProtocol {
}

let stream: QUICStreamInstance
if let flowID = connection.knownFlows[streamID] {
if let flowID = connection.knownFlows[streamID.value] {
// The stream id is known. The flow object may still be missing
// if the stream was torn down without clearing `knownFlows`; in
// that case there is no one to deliver the reset to, so drop it.
Expand Down Expand Up @@ -1136,7 +1136,7 @@ struct FrameResetStream: ~Copyable, QUICFrameProtocol {
// frame as handled.
return true
}
guard let flowID = connection.knownFlows[streamID],
guard let flowID = connection.knownFlows[streamID.value],
let created = connection.flow(for: flowID)
else {
Logger.proto.error(
Expand Down Expand Up @@ -1286,7 +1286,7 @@ struct FrameStopSending: ~Copyable, QUICFrameProtocol {
return false
}

guard let flowID = connection.knownFlows[streamID] else {
guard let flowID = connection.knownFlows[streamID.value] else {
return true
}
guard let stream = connection.flow(for: flowID) else {
Expand Down
41 changes: 32 additions & 9 deletions Sources/SwiftNetwork/QUIC/TransportParameters.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,28 @@ public enum TransportParameterTypes: UInt64, CaseIterable {
case activeConnectionIDLimit = 14
case initialSCID = 15
case retrySCID = 16

case maxDatagramFrameSize = 32
case minAckDelay = 0xff03_de1a

/* Apple Private Relay custom TP. */
case migrationVersion = 0xff08_0808

/// The index used by `TransportParameters` underlying storage.
var index: Int {
switch self.rawValue {
case 0...16:
return Int(self.rawValue)
case Self.maxDatagramFrameSize.rawValue:
return 17
case Self.minAckDelay.rawValue:
return 18
case Self.migrationVersion.rawValue:
return 19
default:
fatalError("Missing case")
}
}
}

enum TransportParameterDecodeErrors: Int, Error {
Expand Down Expand Up @@ -696,38 +713,44 @@ public struct TransportParameters: PrefixedLoggable {
// the transport parameter can have.
public static let maxUDPPayloadSize = 65527
public static let maxDatagramFrameSize: UInt64 = 65535
private var parameterCollection: [TransportParameterTypes: TransportParameter] = [:]
private var parameters: [TransportParameter?]

init(logPrefixer: LogPrefixer = .init()) {
self.log = logPrefixer
self.parameters = Array(
repeating: nil,
count: TransportParameterTypes.allCases.count
)
}

subscript(_ type: TransportParameterTypes) -> TransportParameter? {
parameterCollection[type]
self.parameters[type.index]
}

mutating func append(_ parameter: TransportParameter) {
parameterCollection[parameter.type] = parameter
self.parameters[parameter.type.index] = parameter
}

mutating func remove(_ parameter: TransportParameter) {
parameterCollection[parameter.type] = nil
self.remove(parameter.type)
}

mutating func remove(_ type: TransportParameterTypes) {
parameterCollection[type] = nil
self.parameters[type.index] = nil
}

mutating func removeAll() {
parameterCollection.removeAll()
for index in self.parameters.indices {
self.parameters[index] = nil
}
}

func serialize(
forEarlyData: Bool = false
) throws(QUICError) -> [UInt8] {
var buffer = [UInt8]()
// N.B.: we shuffle the parameters to randomize the order in which they are serialized.
for parameter in parameterCollection.values.shuffled() {
for parameter in parameters.lazy.compactMap({ $0 }).shuffled() {
if forEarlyData && !parameter.serializeForEarlyData {
continue
}
Expand Down Expand Up @@ -823,7 +846,7 @@ public struct TransportParameters: PrefixedLoggable {
if case .minAckDelay(_, let value) = parameter {
minAckDelay = value
}
parameters.parameterCollection[parameter.type] = parameter
parameters.append(parameter)
}
// minAckDelay must be smaller than maxAckDelay.
// If maxAckDelay wasn't sent, we use the default.
Expand All @@ -842,7 +865,7 @@ public struct TransportParameters: PrefixedLoggable {
// Return the integer value for the specified TransportParameter type
// If the specified type does not have an integer value, the function will fail and error out
func intValue(_ forType: TransportParameterTypes) -> Int {
guard let transportParameter = parameterCollection[forType] else {
guard let transportParameter = self[forType] else {
guard let defaultValue = TransportParameter.defaultValue(forType: forType) else {
fatalError("Parameter not set and no default value provided")
}
Expand Down
26 changes: 26 additions & 0 deletions Tests/SwiftNetworkTests/SwiftNetworkTransportParametersTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//===----------------------------------------------------------------------===//
//
// 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 XCTest
@_spi(Essentials) @_spi(ProtocolProvider) @testable import SwiftNetwork

@available(Network 0.1.0, *)
final class SwiftNetworkTransportParametersTests: XCTestCase {

@rpaulo rpaulo Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this test be part of the QUICTests instead?

func testParameterTypesHaveContiguousIndices() {
// Each value should contribute a unique index, the order doesn't matter. Check
// against 'allCases' as that's guaranteed to have one unique index per element.
let indices = TransportParameterTypes.allCases.map { $0.index }.sorted()
XCTAssertEqual(Array(TransportParameterTypes.allCases.indices), indices)
}
}
Loading