There is an odd compilation issues due to the change in 1.13.0 release. Investigating.

@kean Hey,
You just need to remove the imports of WordPressUI.h from the Objective-C files as they all already import it view the Swift header. This should've been a major change in the WordPressUI framework and not a simply minor update to indicate that. But this is the case right now and that should solve it

@alpavanoglu it looks like it worked, thank you! Should I remove the remaining imports?

#import <WordPressUI/UILabel+SuggestSize.h>
// ...

It will also be good for at least second set of eyes to look at this PR.

I can look over this tomorrow. @kean, do you think it makes sense to try to target Monday's beta?

Did you use some particular media-heavy sites yourself?

I searched for "photos" and "GIFs" and browsed more or less randomly. The worst-case scenario is when you browse a mix of GIFs, regular photos, and avatars. The GIFs have their own cache in the trunk, which keeps growing almost indefinitely. I quickly hit ~1.8 GB or RAM, and then the memory clears after a memory warning. I tested on iPhone 13.

You can look for application_low_memory_warning in the console. I just re-tested it again and got four memory warnings in a span of around 5 minutes. I don't think Watchdog can kill an app when you have a debugger attached. In production, that would've been a crash. The memory warnings are the main thing to look for.

MemoryCache also as expected acts as a default cache for AlamofireImage and WordPressUI

In the trunk, the app uses the default AlamofireImage memory cache with a 100 MB limit. So I actually increased the limit to up to 256 MB for the categories of photos that are loaded using AlamofireImage. But I reduced the combined limit significantly.

I don't expect this fix to be a panacea, but it should at least reduce the number of these worst-case-scenario memory warnings and crashes.

do you think it makes sense to try to target Monday's beta?

I'd say yes. I kept the number of changes to the minimum to reduce the risk of regressions. In the worst-case scenario, this small change may end up being not enough, but it should be an improvement.

To summarize, before, the app had the following caches:

• Alamofire's cache with a 100 MB limit
• UIImageView+Networking with no cache limit (used mainly for avatars)
• AnimatedImageCache that uses 20% of RAM (~400 MB on 2 GB) – some of this code was copied from Nuke btw 😃

Now these caches were replaced by a single cache with a 256 MB limit. The expected result of this change is that it will reduce the number of memory warnings, especially when browsing Reader with a mix of regular images, user avatars, and GIFs.

P.S. The limits are imprecise because it's impossible to actually calculate how much memory an image takes in memory, how much memory is needed to render it on screen, and how often the system clears the textures used for rendering. So think of these numbers as more relative than absolute.

Update: I found three more caches: two in WPAvatarSource and one in WPTableImageSource. I'm unsure if they are as consequential as the ones in the PR. I'll investigate what they are for tomorrow and open another PR if it's worth updating them.

The expected result of this change is that it will reduce the number of memory warnings, especially when browsing Reader with a mix of regular images, user avatars, and GIFs.

Yes, I think this logic makes sense, given lowered limits and a single cache for both images and gifs. I ran more extensive tests today. If Reader encounters heavy GIFs, both solutions will react in the same way and memory quickly jumps into "memory_warning" territory. The difference I noticed is what happens afterward. The new solution releases memory quickly and almost fully after huge spikes, while an older solution keeps the memory footprint high, presumably because of the higher limits and multiple caches. Even without memory warnings, the newer solution does release more memory after any memory spike. You can observe it in memory graphs.

	Before memory warning	After memory warning
Before refactoring
After refactoring

Prior to this change, if you were casually browsing the Reader, you could easily reach 2 GB or higher memory usage, which is likely the reason for the frequent Watchdog terminations.

I was curious how much browsing would lead to an actual crash on my device. So I browsed images and gifs today for about 15 minutes but couldn't make it crash. I'm on an iPhone 11, so it has 4GB of RAM. By some estimates, apps can use 60% of device RAM which would be 2.4GB in my case. Since you mention casual browsing could reach 2GB, and I did some intense browsing and didn't see a crash, it's possible that:

• Had I done more browsing, the memory would increase and the app would have been terminated
• Memory is getting managed somehow so it never reaches high enough levels for the app to be terminated

I'm not sure if the app could go beyond the RAM limit by using paged memory, so perhaps 4GB is not the upper limit on my device.

It's clear this PR will help reduce crashes, I wanted to explore how much so, hence the above observations.

I'll prioritize this tomorrow to review since today I only tested the app store version to try to reproduce an actual crash in the Reader.

The following PR addresses the high memory usage when displaying GIFs.

Hey @kean, I don't think we'll get this merged today. It's not a big code change, but as you mention above, it affects images throughout the app, not just Reader (on that note, the PR title seems out of date now).

This is how I see the app's caching right now (based on this Slack thread:

• MemoryCache: The new cache added by this PR
• UIImageView.Downloaded: Folded into MemoryCache in this PR
• AutoPurgingImageCache: Folded into MemoryCache in this PR
• WPTableImageSource: Removed in Remove unused WPTableImageSource and its cache #21152
• WPAvatarSource: Still exists and should be replaced with MemoryCache at some point, as you note here
• SDWebImage: This is still used by the block editor to help with rendering and memory usage.
• Gridicons: Gridicons are bundled with the app so the caching is just to use the same instance of a particular icon instead of a copy

Does that sound accurate to you?

If we're changing the caching of pretty much all the images in the app, can I suggest listing out where in the app (including outside Reader) that you've tested for regressions? It would also be useful to list which parts of the app are not affected by this change. For example, the editor and the avatars shown when @-mentioning someone.

A more detailed breakdown is linked to the recent post on memory caches in Jetpack Mobile.

The list of the affected components is going to be in dozens. My approach was to provide a replacement with the exact same API and behavior (except for the size limits).

I've come back from my vacation. It looks like there has been good progress here. Please let me know if you want me to review and/or test as well.