1772810 - SearchService.jsm is loaded too early during startup

Status: RESOLVED WORKSFORME

(Firefox :: Search, defect, P3)

Description

[Florian Quèze [:florian]]

While working on improving startup times for Firefox 57, we ensured the search service wasn't loaded before first paint.

https://searchfox.org/mozilla-central/rev/4c3f6e8bf87fffb7c62feb4c76a14e0eb0b94c1f/browser/base/content/test/performance/browser_startup.js#59 was meant to prevent this from regressing, but the file has been renamed without renaming the denylist entry.

I just captured a startup profile, and SearchService.jsm is now loaded during early startup, while installing add-ons. Seems to be a regression from bug 1552559.
https://share.firefox.dev/3MgJyXr

SearchSettings.jsm and SearchUtils.jsm are also loaded at that time.

The SearchUtils.jsm load is likely triggered by the loadConsole line at https://searchfox.org/mozilla-central/rev/4c3f6e8bf87fffb7c62feb4c76a14e0eb0b94c1f/toolkit/components/search/SearchService.jsm#1780, causing https://searchfox.org/mozilla-central/rev/4c3f6e8bf87fffb7c62feb4c76a14e0eb0b94c1f/toolkit/components/search/SearchService.jsm#33

The SearchSettings.jsm load is from the SearchService constructor: https://searchfox.org/mozilla-central/rev/4c3f6e8bf87fffb7c62feb4c76a14e0eb0b94c1f/toolkit/components/search/SearchService.jsm#94

I noticed this while reviewing https://phabricator.services.mozilla.com/D148196.

Comment 1

[Mark Banner (:standard8)]

I think it was probably bug 1496075 that regressed this - that one added the dependencies for the add-on manager. The trigger point here is probably the piece of the code where the add-on manager informs the search service about add-ons with search engines, as they are loaded.

Whilst we do try to avoid initing the search service at this stage, xref here and here, we will still load the module files.

I'm not sure there's much we can easily do about it at this stage - the only other real possibility would be to have the add-on manager save the details before invoking the search service, though even then there would still be a possible risk that we'd need to start up early with some add-ons. I'm not sure how much this would actually save us.

I do have a vague idea about not having application provided engines as add-ons, but there could quite easily be other perf issues there and it would only solve the "no search engine add-ons installed" case.

Comment 2

[Mark Banner (:standard8)]

The file reference has been updated, as of a few months back, so I think it is possible that we don't trigger this in our normal test infra, but that it can happen sometimes - maybe depending on how fast the add-on manager can start up.

Maybe we should be checking that the search service hasn't inited at the "before first paint" stage, but maybe allow those modules to load?

Comment 3

[Mark Banner (:standard8)]

We should investigate if there's items that we can delay loading until we really need them - e.g. as referenced in comment 0, SearchSettings is currently being via the SearchService constructor, when it could be delayed until init starts.

We should also see if we can improve the test detection, e.g. check the search service hasn't started initialisation too early.

For user installed add-ons, there might not be much that we can do about the search service starting too early - the add-on manager has to tell us. For the case of application provided engines, we are looking at reworking the search configuration which will also avoid those needing to be defined as add-on engines.

Comment 4

[Mark Banner (:standard8)]

I've taken another look at this now we have the new configuration in place:

• In the case with no search engine add-ons installed, the search service doesn't start until after first paint. This is enforced in the browser_startup.js test, and the module name is correct.
• In the case with a add-on installed which provides a search engine, SearchService.sys.mjs gets loaded by the Add-on Manager to record the fact that it has seen a search add-on on startup. However, initialisation of the search service doesn't start until after the first paint - the same as the first step.

I think that's about the best we can reasonably do. The only other option would be to have the add-on manager save the list of search add-ons on startup, and let the search service pull them from the manager when its ready. I'm not sure that there would be enough benefit here to make it worth the added complexity.

Hence I'm marking this as works for me, as I don't think there's anything else to do here now.