From 281ba67ffccf9aa8a2b6add32fc45496178a424f Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Jan 2021 17:48:54 -0300 Subject: [PATCH] Use separate caches for still and animated media. --- .../ConversationView/CV/CVComponent.swift | 3 +-- .../CVComponents/CVComponentBodyMedia.swift | 3 +-- .../CV/CVComponents/CVComponentSticker.swift | 6 ++--- .../ConversationView/CV/CVItemModel.swift | 3 +-- .../ConversationView/CV/CVLoadContext.swift | 4 +-- .../CV/CVLoadCoordinator.swift | 2 +- .../ConversationView/CV/CVLoader.swift | 4 +-- .../ConversationView/CV/CVNode.swift | 4 +-- .../CV/CVViewStateSnapshot.swift | 4 +-- .../ConversationView/CVViewState.swift | 23 +++------------- .../Cells/CVMediaAlbumView.swift | 4 +-- .../ConversationView/Cells/CVMediaView.swift | 17 +++++------- .../Cells/CVMediaViewCache.swift | 27 ++++++++++++++++++- .../Cells/ReusableMediaView.swift | 15 +++++++---- .../ConversationViewController.m | 9 +++---- 15 files changed, 65 insertions(+), 63 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVComponent.swift b/Signal/src/ViewControllers/ConversationView/CV/CVComponent.swift index c6e0780e20..bfcfbac963 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVComponent.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVComponent.swift @@ -205,8 +205,7 @@ extension CVComponentBase: CVNode { public var itemViewState: CVItemViewState { itemModel.itemViewState } public var messageCellType: CVMessageCellType { componentState.messageCellType } public var conversationStyle: ConversationStyle { itemModel.conversationStyle } - public var cellMediaCache: NSCache { itemModel.cellMediaCache } - public var mediaViewCache: CVMediaViewCache { itemModel.mediaViewCache } + public var mediaCache: CVMediaCache { itemModel.mediaCache } public var isDarkThemeEnabled: Bool { conversationStyle.isDarkThemeEnabled } public var isGroupThread: Bool { diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentBodyMedia.swift b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentBodyMedia.swift index 1e9c73b2d4..e21c8807de 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentBodyMedia.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentBodyMedia.swift @@ -66,8 +66,7 @@ public class CVComponentBodyMedia: CVComponentBase, CVComponent { let conversationStyle = self.conversationStyle let albumView = componentView.albumView - albumView.configure(mediaCache: self.cellMediaCache, - mediaViewCache: self.mediaViewCache, + albumView.configure(mediaCache: self.mediaCache, items: self.items, isOutgoing: self.isOutgoing, isBorderless: self.isBorderless, diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSticker.swift b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSticker.swift index e41845a666..b00edb53ec 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSticker.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSticker.swift @@ -51,12 +51,12 @@ public class CVComponentSticker: CVComponentBase, CVComponent { let cacheKey = attachmentStream.uniqueId let isAnimated = attachmentStream.shouldBeRenderedByYY let reusableMediaView: ReusableMediaView - if let cachedView = mediaViewCache.getMediaView(cacheKey, isAnimated: isAnimated) { + if let cachedView = mediaCache.getMediaView(cacheKey, isAnimated: isAnimated) { reusableMediaView = cachedView } else { let mediaViewAdapter = MediaViewAdapterSticker(attachmentStream: attachmentStream) - reusableMediaView = ReusableMediaView(mediaViewAdapter: mediaViewAdapter, mediaCache: cellMediaCache) - mediaViewCache.setMediaView(reusableMediaView, forKey: cacheKey, isAnimated: isAnimated) + reusableMediaView = ReusableMediaView(mediaViewAdapter: mediaViewAdapter, mediaCache: mediaCache) + mediaCache.setMediaView(reusableMediaView, forKey: cacheKey, isAnimated: isAnimated) } reusableMediaView.owner = componentView diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVItemModel.swift b/Signal/src/ViewControllers/ConversationView/CV/CVItemModel.swift index dbc76fa4d4..8e4d674078 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVItemModel.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVItemModel.swift @@ -28,8 +28,7 @@ public class CVItemModel: NSObject { // and request a new one. private let coreState: CVCoreState public var conversationStyle: ConversationStyle { coreState.conversationStyle } - public var cellMediaCache: NSCache { coreState.cellMediaCache } - public var mediaViewCache: CVMediaViewCache { coreState.mediaViewCache } + public var mediaCache: CVMediaCache { coreState.mediaCache } public var itemModel: CVItemModel { self diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVLoadContext.swift b/Signal/src/ViewControllers/ConversationView/CV/CVLoadContext.swift index c6d41868b4..b7ab276c85 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVLoadContext.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVLoadContext.swift @@ -61,7 +61,7 @@ extension CVItemBuildingContext { var thread: TSThread { threadViewModel.threadRecord } var threadUniqueId: String { thread.uniqueId } var conversationStyle: ConversationStyle { viewStateSnapshot.conversationStyle } - var cellMediaCache: NSCache { viewStateSnapshot.cellMediaCache } + var mediaCache: CVMediaCache { viewStateSnapshot.mediaCache } } // MARK: - @@ -87,7 +87,7 @@ extension CVItemBuilding { var thread: TSThread { itemBuildingContext.thread } var viewStateSnapshot: CVViewStateSnapshot { itemBuildingContext.viewStateSnapshot } var conversationStyle: ConversationStyle { itemBuildingContext.conversationStyle } - var cellMediaCache: NSCache { itemBuildingContext.cellMediaCache } + var mediaCache: CVMediaCache { itemBuildingContext.mediaCache } var transaction: SDSAnyReadTransaction { itemBuildingContext.transaction } var avatarBuilder: CVAvatarBuilder { itemBuildingContext.avatarBuilder } } diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVLoadCoordinator.swift b/Signal/src/ViewControllers/ConversationView/CV/CVLoadCoordinator.swift index ccbba4dc34..a950e6e23b 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVLoadCoordinator.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVLoadCoordinator.swift @@ -61,7 +61,7 @@ public class CVLoadCoordinator: NSObject { private weak var componentDelegate: CVComponentDelegate? private let viewState: CVViewState - private var cellMediaCache: NSCache { viewState.cellMediaCache } + private var mediaCache: CVMediaCache { viewState.mediaCache } private let threadUniqueId: String diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVLoader.swift b/Signal/src/ViewControllers/ConversationView/CV/CVLoader.swift index 9d10fe36d5..d0f26a03ea 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVLoader.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVLoader.swift @@ -249,13 +249,11 @@ public class CVLoader: NSObject { thread: TSThread, containerView: UIView, transaction: SDSAnyReadTransaction) -> CVRenderItem? { - let cellMediaCache = NSCache() let conversationStyle = ConversationStyle(type: .`default`, thread: thread, viewWidth: containerView.width) let coreState = CVCoreState(conversationStyle: conversationStyle, - cellMediaCache: cellMediaCache, - mediaViewCache: CVMediaViewCache()) + mediaCache: CVMediaCache()) return CVLoader.buildStandaloneRenderItem(interaction: interaction, thread: thread, coreState: coreState, diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVNode.swift b/Signal/src/ViewControllers/ConversationView/CV/CVNode.swift index 55bab6fe55..e46c3f2f27 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVNode.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVNode.swift @@ -1,5 +1,5 @@ // -// Copyright (c) 2020 Open Whisper Systems. All rights reserved. +// Copyright (c) 2021 Open Whisper Systems. All rights reserved. // import Foundation @@ -57,7 +57,7 @@ public protocol CVNode { var interaction: TSInteraction { get } var messageCellType: CVMessageCellType { get } var conversationStyle: ConversationStyle { get } - var cellMediaCache: NSCache { get } + var mediaCache: CVMediaCache { get } } // MARK: - diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVViewStateSnapshot.swift b/Signal/src/ViewControllers/ConversationView/CV/CVViewStateSnapshot.swift index 251b371f44..84d4b232f7 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVViewStateSnapshot.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVViewStateSnapshot.swift @@ -1,5 +1,5 @@ // -// Copyright (c) 2020 Open Whisper Systems. All rights reserved. +// Copyright (c) 2021 Open Whisper Systems. All rights reserved. // import Foundation @@ -27,7 +27,7 @@ struct CVViewStateSnapshot { // and request a new one. let coreState: CVCoreState public var conversationStyle: ConversationStyle { coreState.conversationStyle } - public var cellMediaCache: NSCache { coreState.cellMediaCache } + public var mediaCache: CVMediaCache { coreState.mediaCache } // TODO: We need to determine exactly what the desired behavior here is. let collapseCutoffDate = Date() diff --git a/Signal/src/ViewControllers/ConversationView/CVViewState.swift b/Signal/src/ViewControllers/ConversationView/CVViewState.swift index d1fbb4304a..74e1c7782a 100644 --- a/Signal/src/ViewControllers/ConversationView/CVViewState.swift +++ b/Signal/src/ViewControllers/ConversationView/CVViewState.swift @@ -103,7 +103,7 @@ public class CVViewState: NSObject { public var sendMessageController: SendMessageController? - public let mediaViewCache = CVMediaViewCache() + public let mediaCache = CVMediaCache() // MARK: - Gestures @@ -116,14 +116,6 @@ public class CVViewState: NSObject { // MARK: - - @objc - public let cellMediaCache: NSCache = { - let cache = NSCache() - // Cache the cell media for ~24 cells. - cache.countLimit = 24 - return cache - }() - var initialScrollState: CVInitialScrollState? @objc @@ -253,7 +245,7 @@ public extension ConversationViewController { var isMeasuringKeyboardHeight: Bool { inputToolbar?.isMeasuringKeyboardHeight ?? false } - var mediaViewCache: CVMediaViewCache { viewState.mediaViewCache } + var mediaCache: CVMediaCache { viewState.mediaCache } // MARK: - Gestures @@ -288,10 +280,6 @@ public extension ConversationViewController { // MARK: - - var cellMediaCache: NSCache { - viewState.cellMediaCache - } - #if TESTABLE_BUILD var initialLoadBenchSteps: BenchSteps { viewState.initialLoadBenchSteps } #endif @@ -302,9 +290,7 @@ public extension ConversationViewController { extension CVViewState { var asCoreState: CVCoreState { - CVCoreState(conversationStyle: conversationStyle, - cellMediaCache: cellMediaCache, - mediaViewCache: mediaViewCache) + CVCoreState(conversationStyle: conversationStyle, mediaCache: mediaCache) } } @@ -355,8 +341,7 @@ extension ConversationViewController { // pieces of CVC state during async loads. struct CVCoreState { let conversationStyle: ConversationStyle - let cellMediaCache: NSCache - let mediaViewCache: CVMediaViewCache + let mediaCache: CVMediaCache } // MARK: - diff --git a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaAlbumView.swift b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaAlbumView.swift index 5f76a3da53..883e5d49c6 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaAlbumView.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaAlbumView.swift @@ -24,8 +24,7 @@ public class CVMediaAlbumView: UIStackView { super.init(frame: .zero) } - public func configure(mediaCache: NSCache, - mediaViewCache: CVMediaViewCache, + public func configure(mediaCache: CVMediaCache, items: [CVMediaAlbumItem], isOutgoing: Bool, isBorderless: Bool, @@ -40,7 +39,6 @@ public class CVMediaAlbumView: UIStackView { self.items = items self.itemViews = CVMediaAlbumView.itemsToDisplay(forItems: items).map { CVMediaView(mediaCache: mediaCache, - mediaViewCache: mediaViewCache, attachment: $0.attachment, isOutgoing: isOutgoing, maxMessageWidth: maxMessageWidth, diff --git a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift index c6f41096b8..57e99ade63 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift @@ -14,8 +14,7 @@ public class CVMediaView: UIView { // MARK: - - private let mediaViewCache: CVMediaViewCache - private let mediaCache: NSCache + private let mediaCache: CVMediaCache public let attachment: TSAttachment private let conversationStyle: ConversationStyle private let isOutgoing: Bool @@ -26,14 +25,12 @@ public class CVMediaView: UIView { // MARK: - Initializers - public required init(mediaCache: NSCache, - mediaViewCache: CVMediaViewCache, + public required init(mediaCache: CVMediaCache, attachment: TSAttachment, isOutgoing: Bool, maxMessageWidth: CGFloat, isBorderless: Bool, conversationStyle: ConversationStyle) { - self.mediaViewCache = mediaViewCache self.mediaCache = mediaCache self.attachment = attachment self.isOutgoing = isOutgoing @@ -146,7 +143,7 @@ public class CVMediaView: UIView { private func createNewReusableMediaView(mediaViewAdapter: MediaViewAdapter, isAnimated: Bool) { let reusableMediaView = ReusableMediaView(mediaViewAdapter: mediaViewAdapter, mediaCache: mediaCache) - mediaViewCache.setMediaView(reusableMediaView, forKey: mediaViewAdapter.cacheKey, isAnimated: isAnimated) + mediaCache.setMediaView(reusableMediaView, forKey: mediaViewAdapter.cacheKey, isAnimated: isAnimated) applyReusableMediaView(reusableMediaView) } @@ -163,7 +160,7 @@ public class CVMediaView: UIView { // cachekey to avoid conflicts with the actual attachment contents. let cacheKey = blurHash let isAnimated = false - if let reusableMediaView = mediaViewCache.getMediaView(cacheKey, isAnimated: isAnimated) { + if let reusableMediaView = mediaCache.getMediaView(cacheKey, isAnimated: isAnimated) { applyReusableMediaView(reusableMediaView) return } @@ -175,7 +172,7 @@ public class CVMediaView: UIView { private func configureForAnimatedImage(attachmentStream: TSAttachmentStream) { let cacheKey = attachmentStream.uniqueId let isAnimated = attachmentStream.shouldBeRenderedByYY - if let reusableMediaView = mediaViewCache.getMediaView(cacheKey, isAnimated: isAnimated) { + if let reusableMediaView = mediaCache.getMediaView(cacheKey, isAnimated: isAnimated) { applyReusableMediaView(reusableMediaView) return } @@ -187,7 +184,7 @@ public class CVMediaView: UIView { private func configureForStillImage(attachmentStream: TSAttachmentStream) { let cacheKey = attachmentStream.uniqueId let isAnimated = attachmentStream.shouldBeRenderedByYY - if let reusableMediaView = mediaViewCache.getMediaView(cacheKey, isAnimated: isAnimated) { + if let reusableMediaView = mediaCache.getMediaView(cacheKey, isAnimated: isAnimated) { applyReusableMediaView(reusableMediaView) return } @@ -199,7 +196,7 @@ public class CVMediaView: UIView { private func configureForVideo(attachmentStream: TSAttachmentStream) { let cacheKey = attachmentStream.uniqueId let isAnimated = attachmentStream.shouldBeRenderedByYY - if let reusableMediaView = mediaViewCache.getMediaView(cacheKey, isAnimated: isAnimated) { + if let reusableMediaView = mediaCache.getMediaView(cacheKey, isAnimated: isAnimated) { applyReusableMediaView(reusableMediaView) return } diff --git a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaViewCache.swift b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaViewCache.swift index 8367cec615..9daa9f4739 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaViewCache.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaViewCache.swift @@ -5,7 +5,17 @@ import Foundation @objc -public class CVMediaViewCache: NSObject { +public class CVMediaCache: NSObject { + + private static func buildMediaCache(countLimit: Int) -> NSCache { + let cache = NSCache() + cache.countLimit = countLimit + return cache + } + + private let stillMediaCache = buildMediaCache(countLimit: 16) + private let animatedMediaCache = buildMediaCache(countLimit: 8) + private let stillMediaViewCache = MediaInnerCache(maxSize: 12) private let animatedMediaViewCache = MediaInnerCache(maxSize: 6) @@ -23,6 +33,18 @@ public class CVMediaViewCache: NSObject { object: nil) } + @objc + public func getMedia(_ key: String, isAnimated: Bool) -> AnyObject? { + let cache = isAnimated ? animatedMediaCache : stillMediaCache + return cache.object(forKey: key as NSString) + } + + @objc + public func setMedia(_ value: AnyObject, forKey key: String, isAnimated: Bool) { + let cache = isAnimated ? animatedMediaCache : stillMediaCache + cache.setObject(value, forKey: key as NSString) + } + @objc public func getMediaView(_ key: String, isAnimated: Bool) -> ReusableMediaView? { let cache = isAnimated ? animatedMediaViewCache : stillMediaViewCache @@ -39,6 +61,9 @@ public class CVMediaViewCache: NSObject { public func removeAllObjects() { AssertIsOnMainThread() + stillMediaCache.removeAllObjects() + animatedMediaCache.removeAllObjects() + stillMediaViewCache.removeAllObjects() animatedMediaViewCache.removeAllObjects() } diff --git a/Signal/src/ViewControllers/ConversationView/Cells/ReusableMediaView.swift b/Signal/src/ViewControllers/ConversationView/Cells/ReusableMediaView.swift index c5f9d5ec8f..4be7992a27 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/ReusableMediaView.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/ReusableMediaView.swift @@ -30,6 +30,7 @@ public protocol MediaViewAdapter { var mediaView: UIView { get } var isLoaded: Bool { get } var cacheKey: String { get } + var isAnimated: Bool { get } func applyMedia(_ media: AnyObject) func unloadMedia() @@ -54,7 +55,7 @@ public enum ReusableMediaError: Error { public class ReusableMediaView: NSObject { private let mediaViewAdapter: MediaViewAdapterSwift - private let mediaCache: NSCache + private let mediaCache: CVMediaCache @objc public var mediaView: UIView { @@ -92,7 +93,7 @@ public class ReusableMediaView: NSObject { @objc public required init(mediaViewAdapter: MediaViewAdapter, - mediaCache: NSCache) { + mediaCache: CVMediaCache) { self.mediaViewAdapter = mediaViewAdapter as! MediaViewAdapterSwift self.mediaCache = mediaCache } @@ -177,7 +178,7 @@ public class ReusableMediaView: NSObject { let mediaViewAdapter = self.mediaViewAdapter let cacheKey = mediaViewAdapter.cacheKey let mediaCache = self.mediaCache - if let media = mediaCache.object(forKey: cacheKey as NSString) { + if let media = mediaCache.getMedia(cacheKey, isAnimated: mediaViewAdapter.isAnimated) { Logger.verbose("media cache hit") loadCompletion(media) return @@ -193,7 +194,7 @@ public class ReusableMediaView: NSObject { } return mediaViewAdapter.loadMedia() }.done(on: .main) { (media: AnyObject) in - mediaCache.setObject(media, forKey: cacheKey as NSString) + mediaCache.setMedia(media, forKey: cacheKey, isAnimated: mediaViewAdapter.isAnimated) loadCompletion(media) }.catch(on: .main) { (error: Error) in @@ -213,6 +214,7 @@ public class ReusableMediaView: NSObject { class MediaViewAdapterBlurHash: MediaViewAdapterSwift { + public let isAnimated = false let blurHash: String let imageView = UIImageView() @@ -262,6 +264,7 @@ class MediaViewAdapterBlurHash: MediaViewAdapterSwift { class MediaViewAdapterAnimated: MediaViewAdapterSwift { + public let isAnimated = true let attachmentStream: TSAttachmentStream let imageView = YYAnimatedImageView() @@ -315,6 +318,7 @@ class MediaViewAdapterAnimated: MediaViewAdapterSwift { class MediaViewAdapterStill: MediaViewAdapterSwift { + public let isAnimated = false let attachmentStream: TSAttachmentStream let imageView = UIImageView() @@ -373,6 +377,7 @@ class MediaViewAdapterStill: MediaViewAdapterSwift { class MediaViewAdapterVideo: MediaViewAdapterSwift { + public let isAnimated = false let attachmentStream: TSAttachmentStream let imageView = UIImageView() @@ -427,7 +432,7 @@ class MediaViewAdapterVideo: MediaViewAdapterSwift { @objc public class MediaViewAdapterSticker: NSObject, MediaViewAdapterSwift { - let isAnimated: Bool + public let isAnimated: Bool let attachmentStream: TSAttachmentStream let imageView: UIImageView diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 313a0fdba0..0904628873 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -565,8 +565,7 @@ typedef enum : NSUInteger { - (void)applicationDidEnterBackground:(NSNotification *)notification { [self updateCellsVisible]; - [self.cellMediaCache removeAllObjects]; - [self.mediaViewCache removeAllObjects]; + [self.mediaCache removeAllObjects]; } - (void)applicationWillResignActive:(NSNotification *)notification @@ -576,8 +575,7 @@ typedef enum : NSUInteger { self.isWaitingForDeceleration = NO; [self saveDraft]; [self markVisibleMessagesAsRead]; - [self.cellMediaCache removeAllObjects]; - [self.mediaViewCache removeAllObjects]; + [self.mediaCache removeAllObjects]; [self cancelReadTimer]; [self dismissPresentedViewControllerIfNecessary]; [self saveLastVisibleSortIdAndOnScreenPercentage]; @@ -1067,8 +1065,7 @@ typedef enum : NSUInteger { [self saveDraft]; [self markVisibleMessagesAsRead]; [self cancelVoiceMemo]; - [self.cellMediaCache removeAllObjects]; - [self.mediaViewCache removeAllObjects]; + [self.mediaCache removeAllObjects]; [self.inputToolbar clearDesiredKeyboard]; self.isUserScrolling = NO;