Closed Bug 1395265 Opened 2 years ago Closed 2 years ago
Slow performance opening home panel
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.
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.).
Summary: Very slow startup → Slow performance opening home panel
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.
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)
master https://github.com/mozilla-mobile/firefox-ios/commit/f43d0def5109d4c99f67d0f7058a36c3d590244a uplifted to v9.x
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.