Open Bug 2004778 Opened 1 day ago Updated 1 hour ago

Consider delaying mobile `extensions-late-startup` notification to improve Fenix applink startup

Categories

(WebExtensions :: General, enhancement, P3)

All
Android
enhancement

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: acreskey, Assigned: acreskey)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

The extensions-late-startup notification is sent via InitLater (DelayedInit) after the main thread is idle after startup.

A prototype change which further delays the extensions-late-startup notification by 2000ms is showing a large improvement in Fenix applink startup times, about 200ms, almost 10% on A55: perf compare

Profile before: https://share.firefox.dev/3ML5hMQ
Profile after: https://share.firefox.dev/44PmTNR

This is deferring the WebExtension work further down the line so that it's no longer running in parallel with the applink navigation.

Let's see if we can find a solution that improves the applink critical path performance without introducing issues with web extensions.

Related work is ongoing in bug 1958327 - aiming to preallocate the WebExtension process in order to minimize the impact on the applink navigation.

Applying this same test 200ms deferral of extension-late-startup on top of the WebExtension pre-allocation patch yields slightly less of an improvement, but still about a ~170ms, 7.5% improvement to the mean for A55, and ~135ms, ~8% to the mean for Pixel 6.

Perf compare with WebExtension PreAllocate

Before: https://share.firefox.dev/44bYTEy
After: https://share.firefox.dev/4pRjXbx

I'm moving this bug to WebExtensions, even though the trigger for this notification is in GeckoView code at https://searchfox.org/firefox-main/rev/d4456d810664a2a13a871a68e5323522a78c1131/mobile/shared/chrome/geckoview/geckoview.js#896-900 because the WebExtensions engineering team is in the best position to evaluate this request, because the notification's only use is in WebExtensions code.

Copy-pasting my response from Slack, for future reference:

The extensions-late-startup notification for non-desktop (mobile) code exists because the one used on desktop (sessionstore-windows-restored) does not exist on mobile. extensions-late-startup is triggered where it is on mobile for the lack of anything better. Delaying by a short while may be feasible, 2 seconds sounds a lot though.

Does the first navigation depend on the web extensions being init?

It can be, if there is an extension that is registered to block a network request (e.g. an ad blocker). This registration depends on the extension to have started at least once before, so that the extension framework can detect and persist the registration. When such a request and registration is detected, the extension framework will not wait for the extensions-late-startup notification and immediately proceed to start the extension's background script and block the network request until that has completed.

Would delaying the extensions-late-startup notification interfere with how the extension framework persists the registration request?
Or, once registered, does it not matter?

Delaying the notification is going to postpone the startup of most extensions.

On Android especially, extensions are only installed if a user installed it before on demand, during a browser session. On first install, the extension's background script will already start up and trigger the relevant logic to persist event registrations.

Some (too many) extensions are not designed to be event-based, and expect to be alive continuously after startup. Without startup-blocking event listeners (i.e. network blocking), these extensions won't function correctly until the extensions-late-startup notification has been triggered. (but as said before, extensions that are registered to block network requests will work independently of when the extensions-late-startup notification gets triggered).

(Now my question plus answer from Andrew)

Is applink navigation regular browsing? If it is distinct enough I think it may be worth exploring postponing the notification for a short time (e.g. a few 100ms up to maybe even a second or 2 depending on device), if there is a high likelihood of extensions being not relevant for the use case.

Applink navigation is regular browsing except the URI to load comes from an Android intent.
The very specific case that the performance team is focusing on is cold process applink, so where the Fenix processes are spawned explicitly to load a given URI.

Component: Extensions → General
Product: GeckoView → WebExtensions

Some other possible solutions:

  1. Only defer extensions-late-startup notification if we're loading a document on startup browser.webProgress?.isLoadingDocument - patch

  2. Currently we notify extensions-late-startup based on an instantaneous idle being detected. This patch adds a new API which waits for a sustained idle (i.e. main thread idle for 200ms in this case)

These options are being evaluated against our CI tests.

Severity: -- → N/A
Priority: -- → P3

The extensions-late-startup notification can currently fire during applink navigation, causing extension startup to compete with the navigation for resources.

This patch defers the notification to avoid extension startup during applink navigations.

Assignee: nobody → acreskey
Status: NEW → ASSIGNED

I tried a few variations on deferring the extensions-late-startup notification to improve Fenix applink. (These all use web-extension preallocation as a base, bug 1958327). Note that there is a fair deal of noise in these tests.

Defer by 200ms - ~70ms improvement on A55

Defer by 500ms - ~140ms improvement on A55

Defer by 1000ms - ~210ms improvement on A55

Defer until main thread is consistently idle - ~180ms improvement on A55 -- this is interesting but more complicated, propose consider for future

Defer only if geckoview is loading a document - ~165ms improvement on A55 -- but maybe we always want to defer the extension startup?

In the end I put up the simple deferral by 1000ms for review.
But this isn't an area that I work in so happy to hear others' thoughts.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: