From 77c44c323f841e80f458d03a3b2acd286657603c Mon Sep 17 00:00:00 2001 From: Shadowfacts Date: Sun, 11 Sep 2022 22:09:04 -0400 Subject: [PATCH] Use os_unfair_lock for MultiThreadDictionary instead of DispatchQueue --- Tusker/Caching/ImageCache.swift | 4 +- Tusker/MultiThreadDictionary.swift | 81 ++++++++++++++-------- Tusker/Views/AccountDisplayNameLabel.swift | 2 +- Tusker/Views/BaseEmojiLabel.swift | 47 ++++++------- 4 files changed, 80 insertions(+), 54 deletions(-) diff --git a/Tusker/Caching/ImageCache.swift b/Tusker/Caching/ImageCache.swift index c14b172f..57f2dbd6 100644 --- a/Tusker/Caching/ImageCache.swift +++ b/Tusker/Caching/ImageCache.swift @@ -24,7 +24,7 @@ class ImageCache { private let cache: ImageDataCache private let desiredPixelSize: CGSize? - private var groups = MultiThreadDictionary(name: "ImageCache request groups") + private var groups = MultiThreadDictionary() private var backgroundQueue = DispatchQueue(label: "ImageCache completion queue", qos: .default) @@ -96,7 +96,7 @@ class ImageCache { if let data = data { try? self.cache.set(url.absoluteString, data: data, image: image) } - self.groups.removeValueWithoutReturning(forKey: url) + _ = self.groups.removeValue(forKey: url) } groups[url] = group return group diff --git a/Tusker/MultiThreadDictionary.swift b/Tusker/MultiThreadDictionary.swift index f71b4b01..d808bd75 100644 --- a/Tusker/MultiThreadDictionary.swift +++ b/Tusker/MultiThreadDictionary.swift @@ -7,52 +7,79 @@ // import Foundation +import os -class MultiThreadDictionary { - private let name: String - private var dict = [Key: Value]() - private let queue: DispatchQueue +// once we target iOS 16, replace uses of this with OSAllocatedUnfairLock<[Key: Value]> +// to make the lock semantics more clear +@available(iOS, obsoleted: 16.0) +class MultiThreadDictionary { + private let lock: any Lock<[Key: Value]> - init(name: String) { - self.name = name - self.queue = DispatchQueue(label: "MultiThreadDictionary (\(name)) Coordinator", attributes: .concurrent) + init() { + if #available(iOS 16.0, *) { + self.lock = OSAllocatedUnfairLock(initialState: [:]) + } else { + self.lock = UnfairLock(initialState: [:]) + } } subscript(key: Key) -> Value? { get { - var result: Value? = nil - queue.sync { - result = dict[key] + return lock.withLock { dict in + dict[key] } - return result } set(value) { - queue.async(flags: .barrier) { - self.dict[key] = value + lock.withLock { dict in + dict[key] = value } } } - func removeValueWithoutReturning(forKey key: Key) { - queue.async(flags: .barrier) { - self.dict.removeValue(forKey: key) - } - } - /// If the result of this function is unused, it is preferable to use `removeValueWithoutReturning` as it executes asynchronously and doesn't block the calling thread. func removeValue(forKey key: Key) -> Value? { - var value: Value? = nil - queue.sync(flags: .barrier) { - value = dict.removeValue(forKey: key) + return lock.withLock { dict in + dict.removeValue(forKey: key) } - return value } func contains(key: Key) -> Bool { - var value: Bool! - queue.sync { - value = dict.keys.contains(key) + return lock.withLock { dict in + dict.keys.contains(key) } - return value + } + + func withLock(_ body: @Sendable (inout [Key: Value]) throws -> R) rethrows -> R where R: Sendable { + return try lock.withLock(body) + } +} + +// TODO: replace this only with OSAllocatedUnfairLock +@available(iOS, obsoleted: 16.0) +fileprivate protocol Lock { + associatedtype State + func withLock(_ body: @Sendable (inout State) throws -> R) rethrows -> R where R: Sendable +} + +@available(iOS 16.0, *) +extension OSAllocatedUnfairLock: Lock { +} + +// from http://www.russbishop.net/the-law +fileprivate class UnfairLock: Lock { + private var lock: UnsafeMutablePointer + private var state: State + init(initialState: State) { + self.state = initialState + self.lock = .allocate(capacity: 1) + self.lock.initialize(to: os_unfair_lock()) + } + deinit { + self.lock.deallocate() + } + func withLock(_ body: (inout State) throws -> R) rethrows -> R where R: Sendable { + os_unfair_lock_lock(lock) + defer { os_unfair_lock_unlock(lock) } + return try body(&state) } } diff --git a/Tusker/Views/AccountDisplayNameLabel.swift b/Tusker/Views/AccountDisplayNameLabel.swift index 452320fe..c5fd76a6 100644 --- a/Tusker/Views/AccountDisplayNameLabel.swift +++ b/Tusker/Views/AccountDisplayNameLabel.swift @@ -36,7 +36,7 @@ struct AccountDisplayNameLabel: View { let matches = emojiRegex.matches(in: account.displayName, options: [], range: fullRange) guard !matches.isEmpty else { return } - let emojiImages = MultiThreadDictionary(name: "AcccountDisplayNameLabel Emoji Images") + let emojiImages = MultiThreadDictionary() let group = DispatchGroup() diff --git a/Tusker/Views/BaseEmojiLabel.swift b/Tusker/Views/BaseEmojiLabel.swift index 7e53818a..5bac38a5 100644 --- a/Tusker/Views/BaseEmojiLabel.swift +++ b/Tusker/Views/BaseEmojiLabel.swift @@ -38,10 +38,7 @@ extension BaseEmojiLabel { return } - // not using a MultiThreadDictionary so that cached images can be added immediately - // without jumping through various queues so that we can use them immediately - // in building either the final string or the string with placeholders - var emojiImages: [String: UIImage] = [:] + let emojiImages = MultiThreadDictionary() var foundEmojis = false let group = DispatchGroup() @@ -71,12 +68,8 @@ extension BaseEmojiLabel { group.leave() return } - // sync back to the main thread to add the dictionary - // todo: using the main thread for this isn't great - DispatchQueue.main.async { - emojiImages[emoji.shortcode] = transformedImage - group.leave() - } + emojiImages[emoji.shortcode] = transformedImage + group.leave() } if let request = request { emojiRequests.append(request) @@ -92,21 +85,27 @@ extension BaseEmojiLabel { func buildStringWithEmojisReplaced(usePlaceholders: Bool) -> NSAttributedString { let mutAttrString = NSMutableAttributedString(attributedString: attributedString) - // replaces the emojis starting from the end of the string as to not alter the indices of preceeding emojis - for match in matches.reversed() { - let shortcode = (attributedString.string as NSString).substring(with: match.range(at: 1)) - let attachment: NSTextAttachment - - if let emojiImage = emojiImages[shortcode] { - attachment = NSTextAttachment(emojiImage: emojiImage, in: self.emojiFont, with: self.emojiTextColor) - } else if usePlaceholders { - attachment = NSTextAttachment(emojiPlaceholderIn: self.emojiFont) - } else { - continue + // lock once for the entire loop, rather than lock/unlocking for each iteration to do the lookup + // OSAllocatedUnfairLock.withLock expects a @Sendable closure, so this warns about captures of non-sendable types (attribute dstrings, text checking results) + // even though the closures is invoked on the same thread that withLock is called, so it's unclear why it needs to be @Sendable (FB11494878) + // so, just ignore the warnings + emojiImages.withLock { emojiImages in + // replaces the emojis starting from the end of the string as to not alter the indices of preceeding emojis + for match in matches.reversed() { + let shortcode = (attributedString.string as NSString).substring(with: match.range(at: 1)) + let attachment: NSTextAttachment + + if let emojiImage = emojiImages[shortcode] { + attachment = NSTextAttachment(emojiImage: emojiImage, in: self.emojiFont, with: self.emojiTextColor) + } else if usePlaceholders { + attachment = NSTextAttachment(emojiPlaceholderIn: self.emojiFont) + } else { + continue + } + + let attachmentStr = NSAttributedString(attachment: attachment) + mutAttrString.replaceCharacters(in: match.range, with: attachmentStr) } - - let attachmentStr = NSAttributedString(attachment: attachment) - mutAttrString.replaceCharacters(in: match.range, with: attachmentStr) } return mutAttrString