-
Notifications
You must be signed in to change notification settings - Fork 52
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.
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
Adopt Mutex where available #70
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
{ | ||
"mallocCountTotal" : 14 | ||
"mallocCountTotal" : 13 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
{ | ||
"mallocCountTotal" : 14 | ||
"mallocCountTotal" : 13 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
{ | ||
"mallocCountTotal" : 14 | ||
"mallocCountTotal" : 13 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,10 @@ | |
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#if compiler(>=6.0) | ||
import Synchronization | ||
#endif // compiler(>=6.0) | ||
|
||
/// A collection of HTTP fields. It is used in `HTTPRequest` and `HTTPResponse`, and can also be | ||
/// used as HTTP trailer fields. | ||
/// | ||
|
@@ -21,13 +25,19 @@ | |
/// `HTTPFields` adheres to modern HTTP semantics. In particular, the "Cookie" request header field | ||
/// is split into separate header fields by default. | ||
public struct HTTPFields: Sendable, Hashable { | ||
private final class _Storage: @unchecked Sendable, Hashable { | ||
private class _Storage: @unchecked Sendable, Hashable { | ||
var fields: [(field: HTTPField, next: UInt16)] = [] | ||
var index: [String: (first: UInt16, last: UInt16)]? = [:] | ||
let lock = LockStorage.create(value: ()) | ||
|
||
required init() { | ||
} | ||
|
||
func withLock<Result>(_ body: () throws -> Result) rethrows -> Result { | ||
fatalError() | ||
} | ||
|
||
var ensureIndex: [String: (first: UInt16, last: UInt16)] { | ||
self.lock.withLockedValue { _ in | ||
self.withLock { | ||
if let index = self.index { | ||
return index | ||
} | ||
|
@@ -45,10 +55,10 @@ public struct HTTPFields: Sendable, Hashable { | |
} | ||
} | ||
|
||
func copy() -> _Storage { | ||
let newStorage = _Storage() | ||
func copy() -> Self { | ||
let newStorage = Self() | ||
newStorage.fields = self.fields | ||
self.lock.withLockedValue { _ in | ||
self.withLock { | ||
newStorage.index = self.index | ||
} | ||
return newStorage | ||
|
@@ -99,7 +109,40 @@ public struct HTTPFields: Sendable, Hashable { | |
} | ||
} | ||
|
||
private var _storage = _Storage() | ||
#if compiler(>=6.0) | ||
@available(macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0, *) | ||
private final class _StorageWithMutex: _Storage, @unchecked Sendable { | ||
let mutex = Mutex<Void>(()) | ||
|
||
override func withLock<Result>(_ body: () throws -> Result) rethrows -> Result { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this actually compile without warnings under strict concurrency? The closure that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needing dynamic dispatch is also a little gnarly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think dynamic dispatch is still better than an extra allocation for the lock? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Had to revert 948fe6c due to it breaking Swift 5.9/5.10 |
||
try self.mutex.withLock { _ in | ||
try body() | ||
} | ||
} | ||
} | ||
#endif // compiler(>=6.0) | ||
|
||
private final class _StorageWithNIOLock: _Storage, @unchecked Sendable { | ||
let lock = LockStorage.create(value: ()) | ||
|
||
override func withLock<Result>(_ body: () throws -> Result) rethrows -> Result { | ||
try self.lock.withLockedValue { _ in | ||
try body() | ||
} | ||
} | ||
} | ||
|
||
private var _storage = { | ||
#if compiler(>=6.0) | ||
if #available(macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0, *) { | ||
_StorageWithMutex() | ||
} else { | ||
_StorageWithNIOLock() | ||
} | ||
#else // compiler(>=6.0) | ||
_StorageWithNIOLock() | ||
#endif // compiler(>=6.0) | ||
}() | ||
|
||
/// Create an empty list of HTTP fields | ||
public init() {} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how to fix this on Swift 5.9/5.10.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by switching to a compiler version check instead of a canImport check.