Skip to content
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

Fix high memory usage in Reader #21120

Merged
merged 9 commits into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def aztec
end

def wordpress_ui
pod 'WordPressUI', '~> 1.12.5'
pod 'WordPressUI', '~> 1.14'
# pod 'WordPressUI', git: 'https://github.com/wordpress-mobile/WordPressUI-iOS', tag: ''
# pod 'WordPressUI', git: 'https://github.com/wordpress-mobile/WordPressUI-iOS', branch: ''
# pod 'WordPressUI', git: 'https://github.com/wordpress-mobile/WordPressUI-iOS', commit: ''
Expand Down
8 changes: 4 additions & 4 deletions Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ PODS:
- WordPressShared (~> 2.0-beta)
- wpxmlrpc (~> 0.10)
- WordPressShared (2.2.0)
- WordPressUI (1.12.5)
- WordPressUI (1.14.0)
- WPMediaPicker (1.8.8)
- wpxmlrpc (0.10.0)
- Yoga (1.14.0)
Expand Down Expand Up @@ -614,7 +614,7 @@ DEPENDENCIES:
- WordPressAuthenticator (~> 6.3-beta)
- WordPressKit (~> 8.5)
- WordPressShared (~> 2.2)
- WordPressUI (~> 1.12.5)
- WordPressUI (~> 1.14)
- WPMediaPicker (~> 1.8.8)
- Yoga (from `https://raw.githubusercontent.com/wordpress-mobile/gutenberg-mobile/v1.100.1/third-party-podspecs/Yoga.podspec.json`)
- ZendeskSupportSDK (= 5.3.0)
Expand Down Expand Up @@ -882,7 +882,7 @@ SPEC CHECKSUMS:
WordPressAuthenticator: 9bfb316cb165e1997b73aa94d92fadc991cc9725
WordPressKit: f6943a6e927e9f57bc8793938af1e3a4c3adb614
WordPressShared: 87f3ee89b0a3e83106106f13a8b71605fb8eb6d2
WordPressUI: c5be816f6c7b3392224ac21de9e521e89fa108ac
WordPressUI: 09b5477f8678446f27e651cccb6b6401bd19d9a3
WPMediaPicker: 0d40b8d66b6dfdaa2d6a41e3be51249ff5898775
wpxmlrpc: 68db063041e85d186db21f674adf08d9c70627fd
Yoga: 5e12f4deb20582f86f6323e1cdff25f07afc87f6
Expand All @@ -895,6 +895,6 @@ SPEC CHECKSUMS:
ZendeskSupportSDK: 3a8e508ab1d9dd22dc038df6c694466414e037ba
ZIPFoundation: ae5b4b813d216d3bf0a148773267fff14bd51d37

PODFILE CHECKSUM: bc29b8c7c29da0feaa4a5ed3dff2a805e1a06a93
PODFILE CHECKSUM: e815177754512909cfd69c325e5898954f2e6525

COCOAPODS: 1.12.1
1 change: 1 addition & 0 deletions WordPress/Classes/System/WordPressAppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class WordPressAppDelegate: UIResponder, UIApplicationDelegate {
func application(_ application: UIApplication, willFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]? = nil) -> Bool {
window = UIWindow(frame: UIScreen.main.bounds)
AppAppearance.overrideAppearance()
MemoryCache.shared.register()

// Start CrashLogging as soon as possible (in case a crash happens during startup)
try? loggingStack.start()
Expand Down
1 change: 0 additions & 1 deletion WordPress/Classes/Utility/UIAlertControllerProxy.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#import "UIAlertControllerProxy.h"
#import <WordPressUI/WordPressUI.h>
#import "WordPress-Swift.h"


Expand Down
1 change: 0 additions & 1 deletion WordPress/Classes/Utility/WPError.m
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#import "WPError.h"
#import "WPAccount.h"
#import <WordPressShared/NSString+XMLExtensions.h>
#import <WordPressUI/WordPressUI.h>
#import <wpxmlrpc/WPXMLRPC.h>
#import "WordPress-Swift.h"

Expand Down
1 change: 0 additions & 1 deletion WordPress/Classes/Utility/WPWebViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#import "Constants.h"
#import "WPError.h"
#import "WPStyleGuide+WebView.h"
#import <WordPressUI/WordPressUI.h>
#import <WordPressShared/UIDevice+Helpers.h>
#import "WordPress-Swift.h"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
#import "BlogService.h"
#import "SVProgressHUD+Dismiss.h"

#import <WordPressUI/WordPressUI.h>

#import "WordPress-Swift.h"


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#import "SharingDetailViewController.h"
#import "SharingAuthorizationHelper.h"
#import <WordPressShared/WPTableViewCell.h>
#import <WordPressUI/WordPressUI.h>
#import "WordPress-Swift.h"


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#import "SVProgressHUD+Dismiss.h"
#import "SharingAuthorizationHelper.h"
#import <WordPressShared/WPTableViewCell.h>
#import <WordPressUI/WordPressUI.h>
#import "WordPress-Swift.h"


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#import "CommentService.h"
#import "CoreDataStack.h"

#import <WordPressUI/WordPressUI.h>
#import "WordPress-Swift.h"


Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#import "MeHeaderView.h"
#import "Blog.h"
#import <WordPressUI/WordPressUI.h>
#import "WordPress-Swift.h"


Expand Down
92 changes: 18 additions & 74 deletions WordPress/Classes/ViewRelated/Media/AnimatedImageCache.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,13 @@ import UIKit
/// AnimatedImageCache is an image + animated gif data cache used in
/// CachedAnimatedImageView. It should be accessed via the `shared` singleton.
///
class AnimatedImageCache {
final class AnimatedImageCache {

// MARK: Singleton

static let shared: AnimatedImageCache = AnimatedImageCache()
private init() {
NotificationCenter.default.addObserver(self,
selector: #selector(AnimatedImageCache.handleMemoryWarning),
name: UIApplication.didReceiveMemoryWarningNotification,
object: nil)
}

private init() {}

// MARK: Private fields

Expand All @@ -23,50 +19,30 @@ class AnimatedImageCache {
return session
}()

fileprivate static var cache: NSCache<AnyObject, AnyObject> = {
let cache = NSCache<AnyObject, AnyObject>()
cache.totalCostLimit = totalCostLimit()

let bcf = ByteCountFormatter()
bcf.allowedUnits = [.useMB]
bcf.countStyle = .file
let cacheSizeinMB = bcf.string(fromByteCount: Int64(totalCostLimit()))

return cache
}()

// MARK: Instance methods

@objc func handleMemoryWarning() {
clearCache()
}

func clearCache() {
AnimatedImageCache.cache.removeAllObjects()
func cacheData(data: Data, url: URL?) {
guard let url else { return }
let key = url.absoluteString + Constants.keyDataSuffix
MemoryCache.shared.setData(data, forKey: key)
}

func cachedData(url: URL?) -> Data? {
guard let key = url else {
return nil
}
return AnimatedImageCache.cache.object(forKey: key as AnyObject) as? Data
guard let url else { return nil }
let key = url.absoluteString + Constants.keyDataSuffix
return MemoryCache.shared.geData(forKey: key)
}

func cacheStaticImage(url: URL?, image: UIImage?) {
guard let url = url,
let image = image else {
return
}
guard let url, let image else { return }
let key = url.absoluteString + Constants.keyStaticImageSuffix
AnimatedImageCache.cache.setObject(image as AnyObject, forKey: key as AnyObject)
MemoryCache.shared.setImage(image, forKey: key)
staskus marked this conversation as resolved.
Show resolved Hide resolved
}

func cachedStaticImage(url: URL?) -> UIImage? {
guard let url = url else {
return nil
}
guard let url else { return nil }
let key = url.absoluteString + Constants.keyStaticImageSuffix
return AnimatedImageCache.cache.object(forKey: key as AnyObject) as? UIImage
return MemoryCache.shared.getImage(forKey: key)
}

func animatedImage(_ urlRequest: URLRequest,
Expand All @@ -81,7 +57,7 @@ class AnimatedImageCache {

let task = session.dataTask(with: urlRequest, completionHandler: { [weak self] (data, response, error) in
//check if view is still here
guard let _ = self else {
guard let self = self else {
return
}
// check if there is an error
Expand All @@ -101,13 +77,8 @@ class AnimatedImageCache {

let staticImage = UIImage(data: data)
if let key = urlRequest.url {
let dataByteCost = AnimatedImageCache.byteCost(for: data)
AnimatedImageCache.cache.setObject(data as NSData, forKey: key as NSURL, cost: dataByteCost)

// Creating a static image from GIF data is an expensive op, so let's try to do it once...
let imageByteCost = AnimatedImageCache.byteCost(for: staticImage)
let imageKey = key.absoluteString + Constants.keyStaticImageSuffix
AnimatedImageCache.cache.setObject(staticImage as AnyObject, forKey: imageKey as AnyObject, cost: imageByteCost)
self.cacheData(data: data, url: key)
self.cacheStaticImage(url: key, image: staticImage)
guarani marked this conversation as resolved.
Show resolved Hide resolved
}
success?(data, staticImage)
})
Expand All @@ -117,38 +88,11 @@ class AnimatedImageCache {
}
}

// MARK: - Private Helpers

private extension AnimatedImageCache {
static func byteCost(for image: UIImage?) -> Int {
guard let image = image, let imageRef = image.cgImage else {
return 0
}
return imageRef.bytesPerRow * imageRef.height // Cost in bytes
}

static func byteCost(for data: Data?) -> Int {
guard let data = data else {
return 0
}
return data.count // Cost in bytes
}

static func totalCostLimit() -> Int {
let physicalMemory = ProcessInfo.processInfo.physicalMemory
let cacheRatio = physicalMemory <= (Constants.memory512MB) ? Constants.smallCacheRatio : Constants.largeCacheRatio
let cacheLimit = physicalMemory / UInt64(1 / cacheRatio)
return cacheLimit > UInt64(Int.max) ? Int.max : Int(cacheLimit)
}
}

// MARK: - Constants

private extension AnimatedImageCache {
struct Constants {
static let keyDataSuffix = "_data"
static let keyStaticImageSuffix = "_static_image"
static let memory512MB: UInt64 = 1024 * 1024 * 512 // 512 Mb
static let smallCacheRatio: CGFloat = 0.1
static let largeCacheRatio: CGFloat = 0.2
}
}
118 changes: 118 additions & 0 deletions WordPress/Classes/ViewRelated/Media/MemoryCache.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import Foundation
import AlamofireImage
import WordPressUI
import SDWebImage

final class MemoryCache {
/// A shared image cache used by the entire system.
static let shared = MemoryCache()

private let cache = NSCache<NSString, AnyObject>()

private init() {
self.cache.totalCostLimit = 256_000_000 // 256 MB

NotificationCenter.default.addObserver(self, selector: #selector(didReceiveMemoryWarning), name: UIApplication.didReceiveMemoryWarningNotification, object: nil)
}

@objc private func didReceiveMemoryWarning() {
cache.removeAllObjects()
}

// MARK: - UIImage

func setImage(_ image: UIImage, forKey key: String) {
cache.setObject(image, forKey: key as NSString, cost: Int(image.sd_memoryCost))
}

func getImage(forKey key: String) -> UIImage? {
cache.object(forKey: key as NSString) as? UIImage
}

func removeImage(forKey key: String) {
cache.removeObject(forKey: key as NSString)
}

// MARK: - Data

func setData(_ data: Data, forKey key: String) {
cache.setObject(data as NSData, forKey: key as NSString, cost: data.count)
}

func geData(forKey key: String) -> Data? {
cache.object(forKey: key as NSString) as? Data
}

func removeData(forKey key: String) {
cache.removeObject(forKey: key as NSString)
}
}

extension MemoryCache {
/// Registers the cache with all the image loading systems used by the app.
func register() {
// WordPressUI
WordPressUI.ImageCache.shared = WordpressUICacheAdapter(cache: .shared)

// AlamofireImage
UIImageView.af_sharedImageDownloader = AlamofireImage.ImageDownloader(
staskus marked this conversation as resolved.
Show resolved Hide resolved
imageCache: AlamofireImageCacheAdapter(cache: .shared)
)

// WordPress.AnimatedImageCache uses WordPress.MemoryCache directly
}
}

private struct WordpressUICacheAdapter: WordPressUI.ImageCaching {
let cache: MemoryCache

func setImage(_ image: UIImage, forKey key: String) {
cache.setImage(image, forKey: key)
}

func getImage(forKey key: String) -> UIImage? {
cache.getImage(forKey: key)
}
}

private struct AlamofireImageCacheAdapter: AlamofireImage.ImageRequestCache {
let cache: MemoryCache

func image(for request: URLRequest, withIdentifier identifier: String?) -> AlamofireImage.Image? {
image(withIdentifier: cacheKey(for: request, identifier: identifier))
}

func add(_ image: AlamofireImage.Image, for request: URLRequest, withIdentifier identifier: String?) {
add(image, withIdentifier: cacheKey(for: request, identifier: identifier))
}

func removeImage(for request: URLRequest, withIdentifier identifier: String?) -> Bool {
removeImage(withIdentifier: cacheKey(for: request, identifier: identifier))
}

func image(withIdentifier identifier: String) -> AlamofireImage.Image? {
cache.getImage(forKey: identifier)
}

func add(_ image: AlamofireImage.Image, withIdentifier identifier: String) {
cache.setImage(image, forKey: identifier)
}

func removeImage(withIdentifier identifier: String) -> Bool {
cache.removeImage(forKey: identifier)
return true
}

func removeAllImages() -> Bool {
// Do nothing (the app decides when to remove images)
return true
}

private func cacheKey(for request: URLRequest, identifier: String?) -> String {
var key = request.url?.absoluteString ?? ""
if let identifier = identifier {
key += "-\(identifier)"
}
return key
}
}
1 change: 0 additions & 1 deletion WordPress/Classes/ViewRelated/Menus/MenusViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#import "WordPress-Swift.h"
#import <WordPressShared/WPFontManager.h>
#import <WordPressShared/WPStyleGuide.h>
#import <WordPressUI/WordPressUI.h>



Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

#import "Media.h"
#import "WordPress-Swift.h"
#import <WordPressUI/WordPressUI.h>


@interface FeaturedImageViewController ()
Expand Down
Loading