Refactor FXIOS-9353 #20714 Improving BVC viewDidLoad #24487

lmarceau · 2025-01-31T17:26:55Z

📜 Tickets

💡 Description

I started profiling the start up of the app, and this is a first attempt at improving the BVC load time since it has a big impact. This is a little step since we gotta start somewhere, and small incremental changes in that area makes more sense to ensure there's no breakage. My intent there is to move some things off the main thread when we first load BVC. I noticed:

We were creating an unused UISearchController under the AppStartUpTelemetry, this had impact on launch time (see screenshot).
I moved the app start up telemetry off the main thread with a delay, since some of those telemetry are accessing RustLogins which seems to be unneeded on the main thread when we're trying to load the app.
Non UI tasks where also moved on a background thread.
updateToolbarStateForTraitCollection was removed from the viewDidLoad since it's also present in viewWillAppear, so that seems like a duplicate call not needed.

📝 Checklist

You have to check all boxes before merging

Filled in the above information (tickets numbers and description of your work)
Updated the PR name to follow our PR naming guidelines
Wrote unit tests and/or ensured the tests suite is passing
When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
If needed, I updated documentation / comments for complex code and public methods
If needed, added a backport comment (example @Mergifyio backport release/v120)

firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift

mattreaganmozilla

I haven't had a chance to exercise this branch thoroughly but the changes look Ok to me 👍 , my only minor concern is that offloading some of this might introduce hard-to-replicate edge cases. Might be a good idea to get a 2nd reviewer sign-off, but otherwise lgtm

+ fix some stuff that needs to be on the main thread

Move telemetry in the telemetry tracking class

- Move Search bar telemetry to the trackStartUpTelemetry function - Move updateCreditCardAutofillStatus into main.async - Remove commented code - Move setupNotifications back to setupEssentialUI since we dont want race conditions

…lemetry queue

lmarceau · 2025-02-04T16:57:18Z

Okkk. I made some updates and removed the non essential UI function (using a DispatchQueue.main.async since it didn't seem to have any positive impact after re-profiling. I also change to use a Task(priority:) instead of a DispatchQueue as suggested.

The only offloading is related to telemetry now. Impact should be minimal. Load time is slightly improved by 15 ms with this PR 🤷‍♀️ I'll take that, better than nothing!

…ozilla-mobile#24487) * Try to improve viewDidLoad * Remove UISearchController init from startpath, wasn't even needed + fix some stuff that needs to be on the main thread * Remove redundant call to updateToolbarStateForTraitCollection Move telemetry in the telemetry tracking class * Update - Move Search bar telemetry to the trackStartUpTelemetry function - Move updateCreditCardAutofillStatus into main.async - Remove commented code - Move setupNotifications back to setupEssentialUI since we dont want race conditions * Create setupNonEssentialUI function * Remove non essential UI + change to Task(priority:) for background telemetry queue

FilippoZazzeroni · 2025-02-05T09:16:56Z

That is good @lmarceau it is still an improvement 🎉

lmarceau requested review from FilippoZazzeroni and mattreaganmozilla January 31, 2025 17:26

lmarceau requested a review from a team as a code owner January 31, 2025 17:26

mattreaganmozilla reviewed Jan 31, 2025

View reviewed changes