Page MenuHomePhabricator

[SPIKE] Load test for article navigation stack
Closed, ResolvedPublic

Description

Profile the app while building an article view stack that's 10-deep (twice the average pageviews/session—IIRC). Be sure to make a note of the "stack" of articles so that comparisons can be made somewhat accurately. I propose we go to "Barack Obama" article, and then navigate to the previous president (via the info box) until the "stack" is 10 deep. Using presidents should ensure the articles are somewhat similar and have at least a few images.

Event Timeline

BGerstle-WMF raised the priority of this task from to Needs Triage.
BGerstle-WMF updated the task description. (Show Details)
JMinor set Security to None.
BGerstle-WMF raised the priority of this task from Medium to High.
BGerstle-WMF lowered the priority of this task from High to Medium.

Good news everyone! We don't appear to have any large leaks when scrolling the feed or pushing/popping articles! The only leaks that showed up in Instruments were minor (<1KB each) and attributed to system frameworks (stack traces don't lead to any app code):

Leaked Object	#	Address	Size	Responsible Library	Responsible Frame
NSMutableArray	3	< multiple >	144 Bytes	UIKit	-[_UIKeyboardTextSelectionGestureController init]
_UIKeyboardBasedNonEditableTextSelectionGestureController	4	< multiple >	640 Bytes	UIKit	-[UIWebSelectionAssistant addNonEditableForceTextSelectionGestureRecognizersToView:]
_UIKeyboardTextSelectionController	4	< multiple >	384 Bytes	UIKit	-[UIWebSelectionAssistant addNonEditableForceTextSelectionGestureRecognizersToView:]

In other news, here are some allocations results for a run on iOS 9. During this run, I did the following:

  • Launched the app
  • Navigated to TFA "Youth on the Prow, and Pleasure at the Helm"
  • Clicked link to "Oil Painting"
  • Clicked link to "Painting"
  • Clicked link to "Pigment"
  • Popped each article, then dismissed browser view

All Allocations

Graph	 Category	# Persistent	Persistent Bytes	# Transient	Total Bytes	# Total	Transient/Total Bytes	
0	VM: CG raster data	79	53.06 MB	70	56.02 MB	149	<XRRatioObject: 0x7fc44f221460>  %0.07, %0.00	
0	Malloc 2.58 MB	7	18.05 MB	0	18.05 MB	7	<XRRatioObject: 0x7fc456a25e60>  %0.02, %0.00	
0	VM: CoreAnimation	52	7.78 MB	116	17.84 MB	168	<XRRatioObject: 0x7fc456a25e60>  %0.01, %0.01	
0	VM: Foundation	27	5.06 MB	42	11.59 MB	69	<XRRatioObject: 0x7fc456a25e60>  %0.01, %0.01	
0	Malloc 672.00 KB	7	4.59 MB	2	5.91 MB	9	<XRRatioObject: 0x7fc4a1879c80>  %0.01, %0.00	
0	Malloc 336.00 KB	7	2.30 MB	1	2.62 MB	8	<XRRatioObject: 0x7fc4a1879c80>  %0.00, %0.00

CG raster data

AKA Cached image bitmaps stored in SDImageCache.

These come from both from regular image requests, but some are being inflated into the memory cache as a result of the new image gallery inclusion logic. This alone makes up the lions share of memory usage overall and persistently. By persistent, I mean these images hang around in memory indefinitely (we get a warning, count/size limit is reached, etc.).

Row 2 (Malloc 2.58 MB)

Whenever we detect faces, it seems CIDetector does some internal allocations that stick around—probably because we're reusing the same detector (see this SO post).

VM Core Animation

Unclear, lots of CALayer CATransaction stuff, probably not anything we can do.

VM Foundation

WMFArticleImageProtocol returning image data to the web view. Unclear why this data is sticking around, but it's possible we need an autoreleasepool or to prevent NSURLCache from holding onto these objects.

Unlisted

I empirically observed our memory "watermark" changing by 5-10MB whenever we pushed/popped an article whose images were cached. Could be due to UIWebView memory usage, but maybe not. Could be worth doing a quick experiment to see how much reusing web views would save us in terms of raw allocations.


Suggested actions

In order of recommended priority:

  1. Explore ways to control image caching persistence
    • Count/cost limits
    • Purging memory for images in an article (immediately|5s) after it is popped off the stack
      • Only caching article images on disk? Article views aren't like tables where the image view is re-set w/ a different image, we might be able to rely on UIWebView keeping the image "alive" in its content, and re-requesting from disk the next time that article is loaded
  2. Explore CPU cost of reusing CIDetector on articles vs. on the feed. Most heavily used on the feed
    • Could also recycle CIDetector singleton on memory warnings, need to watch out for threading though
  3. Investigate WMFArticleImageProtocol memory usage
  4. Investigate potential gains from reusing UIWebView instances

@JMinor I can file tasks for suggested actions, but have a look to review my results. @Mhurd @Fjalapeno please review as well.

Thanks Brian. I'll give the other engineers a chance to read/comment, and then we can decide on plans for the suggested actions.

When you start an update with this:
"Good news everyone!"

I assume it will actually be followed by terrible news. Too much Farnsworth.

Here's the trace I got the above data from:

Note that the different heap generators correspond to:

A: "Feed" has loaded initial cells (some images, etc.)
B: Tapped on and waited for TFA to load ("Youth on the Prow, and Pleasure at the Helm"
C: Tapped on and waited for "Oil Painting" to load
D: Tapped on and waited for "Painting" to load
E: Tapped on and waited for "Pigment" to load
F: Tapped back twice, wound up on "Oil Painting" (meant to only tap once, but Instruments was laggy)
G: Tapped back again, wound up on TFA
H: Closed browser view

Memory Profile Part 2 aka WTF

All Your Memory Is Belong To CoreAnimation

I've run a second test, traversing U.S. Presidents' pages in reverse order on my iPad 3, iOS 8.3. I got to Richard Nixon (8 deep) and encountered two memory warnings. After the last memory warning (on Nixon's page), the app was running at about 267 MB active memory (getting a warning every time we hit about 320MB). At the time of the last warning, the memory was being used by (in order of most to least used):

  1. CoreAnimation storing all the layers of all the views presented up to that point at 68.5 MB
  2. SDWebImage bitmaps at 53.33 MB
  3. JavascriptCore (WTF::*) at 46.96
  4. CIDetector 28.29 MB

There are some others, e.g. JSON data, some GZIP stuff, and additional HTML data in the web view. The best way to view the data is:

  1. Select "Allocations" instrument
  2. Select "Call Trees" from the drop down in the middle toolbar (Fig A)
  3. Select "Invert Call Tree" option in the track display settings (Fig B)

Fig A:

pasted_file (763×639 px, 128 KB)

Fig B:

pasted_file (872×1 px, 235 KB)

Suggested Actions

Listed in order of suggested priority

Purge CIDetector on memory warning

Should be fairly easy to do, gain 28 MB of memory back easily.

Serialize view controllers in stack on memory warning

A bit more difficult, but this should eliminate most persistent memory usage by:

  • CA memory to be released
  • Bitmaps to be released from UIImageViews & web view img elements
  • JavascriptCore memory to be released (or collected, whatever)

Note, this is orthogonal to reusing web views. The idea is to optimize UINavigationController so that it will serialize the bottom N-1 view controllers in its stack to snapshots, and restore them lazily when they are popped (perhaps after a short delay to prevent thrashing when pops happen in quick succession). I think this is more important than figuring out web view reuse because of the large amount of memory being used by CoreAnimation just keeping our views around. Since iOS 6, viewDidUnload is never called, so views are never unloaded from memory, even in low-memory conditions.

Reuse web views

If optimizing UINavigationController turns out to be a dead end, this is our next option. Instead of snapshotting and serializing the entire article view, just snapshot the webview (which is most of the article view anyway, aside from Read More, ToC, etc.).

Reuse auxiliary views

Article views should set their ToC reference to nil on a memory warning if it's not currently presented. This should save some memory easily.

Trace:

Assigning to me to create follow-up tickets and triage.

Spikes:

CIDetector

  • Try reusing CIDetector and measure impact on FPS & image time to screen
  • If reusing cost is too high, do thread-safe CIDetector "caching" and purging on memory warning

SDWebImage bitmaps

  • Set count limit to fault unused images more aggressively, will be reloaded from disk as needed
  • Purge when going to background? (does NSCache do this for us?)

View stack

  • Serializing entire view controllers to keep stack memory usage to a memory
  • Deallocate web view on viewDidDisappear, and recreate on viewWillAppear
  • Spike run memory test w/ web-views disabled to quickly figure out potential gains from focusing on web view memory use

Subtasks have been created (not blocking subtasks though)