Closed Bug 1395265 Opened 2 years ago Closed 2 years ago

Slow performance opening home panel

Categories

(Firefox for iOS :: Browser, enhancement, P1)

x86_64
iOS
enhancement

Tracking

()

RESOLVED FIXED
Iteration:
1.29
Tracking Status
fxios 9.0 ---

People

(Reporter: overholt, Assigned: justindarc)

References

Details

(Keywords: perf, Whiteboard: [MobileCore])

Attachments

(1 file)

I experience very poor performance on my iPhone SE. Opening and closing tabs is very laggy as is the UI getting to the point that I can even type in a URL. Without resorting to hyperbole, it's a very frustrating and sub-optimal experience, especially when (perhaps unfairly?) compared with Safari on the same device.
Keywords: perf
Hardware: Other → x86_64
See Also: → iphone5c-perf
This currently by-design unfortunately, we are bound by performing lots of main-thread I/O on startup, and various UI code waits on this to complete. QE-tested startup times show 4-6 secs cold boot time. Safari is closer 1 sec on my device.

A better app design is to do absolute minimal disk I/O to load bare-bones state info (ideally just NSUserDefault preferences) to get to a web view that loads the last viewed page (which is sort-of the default cold boot app state). Background threads do all the heavy init, and the UI has to be tweaked to survive without all the app backend loaded (grey out some buttons, show spinners if tableviews are shown that don't have data yet, etc.).
Duplicate of this bug: 1396461
Summary: Very slow startup → Slow performance opening home panel
Priority: -- → P2
The new tab profile is the same problem profile attached on 
https://bugzilla.mozilla.org/show_bug.cgi?id=1392679
We have main thread DB writes happening which is a killer.

Richard Newman's comments from Slack on this profile describe 3 areas of how the backend problem is happening

```
it is `HomePanelViewController`.
The stack you posted is `HPVC`, and `HPVC` is the difference between “Blank Page” or any other choice.
So in `HPVC.viewDidLoad` we do a ton of expensive work, and because our writes are synchronous by default, they block the main thread.
As I mentioned above, we (a) should not be doing this work so often (not on every new tab!), (b) should not be doing this work right then, and (c) should not be doing it synchronously.
So something is wrong with the caching, and we can do much better about choosing when to invalidate.
The profile for new tab operation only is this: https://pastebin.mozilla.org/9031529

This is a 2600ms block of samples for when the main-thread CPU is 100% saturated (the 2nd cpu is largely idle at this time), for which 100% of the time is in this single hot path accessing the database.
Because pastebins evaporate:

---
  29 Client 2625.0  BrowserViewController.tabManager(_:didSelectedTabChange:previous:) /Users/garv/code/firefox-ios/Client/Frontend/Browser/BrowserViewController.swift:2222
  28 Client 2625.0  BrowserViewController.updateInContentHomePanel(_:) /Users/garv/code/firefox-ios/Client/Frontend/Browser/BrowserViewController.swift:829
  27 Client 2625.0  BrowserViewController.showHomePanelController(inline:) /Users/garv/code/firefox-ios/Client/Frontend/Browser/BrowserViewController.swift:768
  26 UIKit 2625.0  -[UIViewController view]
  25 UIKit 2625.0  -[UIViewController loadViewIfRequired]
  24 Client 2625.0  @objc HomePanelViewController.viewDidLoad() /Users/garv/code/firefox-ios/Client/Frontend/Home/HomePanelViewController.swift:0
  23 Client 2625.0  HomePanelViewController.viewDidLoad() /Users/garv/code/firefox-ios/Client/Frontend/Home/HomePanelViewController.swift:138
  22 Client 2625.0  protocol witness for DataObserver.refreshIfNeeded(forceHighlights:forceTopSites:) in conformance ActivityStreamDataObserver /Users/garv/code/firefox-ios/Client/Frontend/Home/PanelDataObservers.swift:0
  21 Client 2625.0  ActivityStreamDataObserver.refreshIfNeeded(forceHighlights:forceTopSites:) /Users/garv/code/firefox-ios/Client/Frontend/Home/PanelDataObservers.swift:72
  20 Storage 2625.0  protocol witness for HistoryRecommendations.repopulate(invalidateTopSites:invalidateHighlights:) in conformance SQLiteHistory /Users/garv/code/firefox-ios/Storage/SQL/SQLiteHistoryRecommendations.swift:0
  19 Storage 2625.0  SQLiteHistory.repopulate(invalidateTopSites:invalidateHighlights:) /Users/garv/code/firefox-ios/Storage/SQL/SQLiteHistoryRecommendations.swift:147
  18 Storage 2625.0  BrowserDB.run(_:) /Users/garv/code/firefox-ios/Storage/SQL/BrowserDB.swift:466
  17 Storage 2625.0  BrowserDB.transaction(_:callback:) /Users/garv/code/firefox-ios/Storage/SQL/BrowserDB.swift:314
  16 Storage 2625.0  BrowserDB.transaction(synchronous:err:callback:) /Users/garv/code/firefox-ios/Storage/SQL/BrowserDB.swift:321
  15 Storage 2625.0  SwiftData.transaction(synchronous:transactionClosure:) /Users/garv/code/firefox-ios/Storage/ThirdParty/SwiftData.swift:191
  14 Storage 2625.0  SwiftData.withConnection(_:synchronous:cb:) /Users/garv/code/firefox-ios/Storage/ThirdParty/SwiftData.swift:132
---
The important things to note here:

- `refreshIfNeeded` is being called inside `viewDidLoad`. We should revisit that decision.
- `refreshIfNeeded` is directly calling `SQLiteHistory.repopulate`, and that's doing a synchronous database write. It should not do so.
- If the write begins before a read, it'll still block loading of top sites.
- The UI will need to be ready to repopulate a few seconds later, because sometimes the cache is empty.
Attached file GitHub Pull Request
Here's a PR that should move the AS queries off of `DispatchQueue.main`. There's probably still more we can do here though.
Assignee: nobody → jdarcangelo
Status: NEW → ASSIGNED
Attachment #8904743 - Flags: review?(gkeeley)
Iteration: --- → 1.29
Priority: P2 → P1
Attachment #8904743 - Flags: review?(gkeeley) → review?(fpatel)
Attachment #8904743 - Flags: review?(fpatel) → review+
master https://github.com/mozilla-mobile/firefox-ios/commit/f43d0def5109d4c99f67d0f7058a36c3d590244a
uplifted to v9.x
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Whiteboard: [MobileCore]
You need to log in before you can comment on or make changes to this bug.