mozilla-mobile / firefox-ios

Firefox for iOS
Mozilla Public License 2.0
12.17k stars 2.92k forks source link

CPU Spike - Ingestion suggestions #20289

Open data-sync-user opened 4 months ago

data-sync-user commented 4 months ago

We have a large spike in CPU exceptions that correlates with this experiment. It started around May 1st.

!image-20240516-165948.png|width=1007,height=378,alt="image-20240516-165948.png"!

┆Issue is synchronized with this Jira Task

linabutler commented 4 months ago

I think the "CPU exception" is a diagnostic that's emitted whenever the app is using too much CPU time in the background, and the OS terminates it—it's not a typical user-visible crash.

I think the high CPU usage might be caused by SuggestStore.ingest() blocking the main thread until it's done. ingest() is called from the RustFirefoxSuggest actor, but, per @OrlaM's comment here, actors aren't really appropriate for this—we should be using a dispatch queue instead, and awaiting its result via withCheckedContinuation to make it play well with async/await.

Once we have a SuggestStore.interruptEverything() API (https://bugzilla.mozilla.org/show_bug.cgi?id=1897264) in the Rust component, we can change iOS to:

linabutler commented 4 months ago

Call SuggestStore.ingest() and SuggestStore.query() on a (concurrent) dispatch queue.

Nitpicking my own analysis (🧐): it's almost definitely more efficient to use a pair of serial queues for this (with appropriate QoS), than a concurrent queue. There's a bit more about that captured here.

data-sync-user commented 4 months ago

➤ Norberto Andres Furlan commented:

Lina Butler to confirm if we can have this for FX 127.

CC: Nive Suresh Orla Mitchell

data-sync-user commented 4 months ago

➤ Diana Andreea Barladeanu commented:

Hi, Lina Butler ! What should QA test in this ticket? Thanks!

data-sync-user commented 4 months ago

➤ Lina Butler commented:

Hi Diana Andreea Barladeanu! 👋 I think just a smoke test that enables the Cross-Platform Suggest experiment, and makes sure that the suggestions are eventually ingested and show up in the address bar, would be sufficient for testing. This ticket reworked the implementation, but the user-facing experience should be the same. Thanks!

data-sync-user commented 4 months ago

➤ Diana Andreea Barladeanu commented:

Validated on v127 (42448), with iPhone 15 (17.4.1).

!RPReplay_Final1717484132.mp4|width=590,height=1280,alt="RPReplay_Final1717484132.mp4"!