1567258 - Make Monitor a built-in component

Status: RESOLVED FIXED

(Firefox :: Firefox Monitor, task, P3)

Description

[Andrew Swan [:aswan]]

We're not shipping out-of-cycle updates to Firefox Monitor, so we're paying system addon overhead for it unnecessarily. A first draft patch (that still needs some l10n work) to convert it to a built-in component is here:
https://phabricator.services.mozilla.com/D37457

Lets measure the impact on startup performance and, if it looks good, polish off that patch and get it landed.

Comment 1

[Andrew Swan [:aswan]]

Nihanth, we talked about this a while ago, but just want to confirm: we don't have any specific reasons to keep shipping monitor as an extension right?

Comment 2

[Andrew Swan [:aswan]]

Here are talos comparisons for the patch mentioned above, though there's not a very strong signal:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=5612defaf368e15634a9843df572d53e65b1f6f5&newProject=try&newRevision=d65ba6ebc6371ca103eabeb7940af5b73f987dda

However, we do have some evidence that this would make an impact on the first run (and on runs after upgrades)
This is a profile from a reference device that shows over 1 second spent in XPIProvider.checkForChanges():
https://perfht.ml/2Z032Yl

This function synchronously reads and parses the manifests for all built-in system addons. Monitor is one of 5 system addons that exist at the time this profile was taken, so making monitor built-in would not remove all that overhead but it would take a nice bite out of it.

Comment 3

[(inactive) Nihanth Subramanya [:nhnt11]]

I don't think we have any specific reasons to keep shipping Monitor as an extension. I believe that there are some changes to the doorhanger in the roadmap for the Monitor features in Firefox, and that might be a good opportunity to move the code into browser/base. We definitely don't have cycles to allocate for this work in 70, and probably not in 71 either unless the timeline accelerates. Thanks for filing this and for the talos comparisons + first run profile!

Comment 4

[:Gijs (out for now; he/him)]

Sounds like this is relatively straightforward and offers potential startup improvements esp. on clean profiles.

Comment 5

[Andrew Swan [:aswan]]

Attachment: Bug 1567258 - Convert fxmonitor to a built-in component

Comment 6

[Francesco Lodolo [:flod]]

@aswan
Is this something you still plan to land, or should we try to find a new owner?

Comment 7

[Andrew Swan [:aswan]]

My phabricator account got disabled last week and I'm still trying to get access to it. The requested changes are quite small, if somebody else is available to finish this off, that would be great. Nihanth, any chance you can do it?

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae44ff168518ff19c8224f3f6f20b6f031df5279

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/740a6df70b30
Convert fxmonitor to a built-in component r=nhnt11,flod

Comment 10

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/740a6df70b30