1534371 - Lazy load browser.xul menus

Status: NEW

(Firefox :: Menus, enhancement, P3)

Description

[:Felipe Gomes (needinfo for replies!)]

I sent a patch [1] to tryserver that removes most of the menus from the browser.xul markup to see what impact it would have.

It seems it would give the following improvements [2]:

• 1.2% - 1.5% ts_paint
• 3.0% - 6.8% tpaint

and maybe something for startup_about_home_paint but that was uncertain from this test run.

Note that it's not some trivial work to do this. My patch just tested the removal. Building the menu during the first onpopupshowing is simple, but someone needs to carefully inspect all other code that may be trying to get a direct reference to one of the items inside the menus (including if such code can be triggered by the webextensions API).

[1] https://hg.mozilla.org/try/rev/dc0f744ae5f5edf1639bfbe2e05d9768a7bddd4b
The patch doesn't look that big, but if you look closely, some key "#includes" are removed with it, making it a larger removal.

[2] https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=dc0f744ae5f5edf1639bfbe2e05d9768a7bddd4b&framework=1&selectedTimeRange=172800

Comment 1

[:Gijs (he/him)]

This seems worth pursuing, fxperf:p2 because it would indeed require some non-trivial investment.

Comment 2

[Brian Grinstead [:bgrins]]

FYI I've played around with a couple of variations of this when we were looking into browser.xhtml performance that might be interesting.

1. Here's an example where I store popupset contents in a separate xml file, load it as a fragment in a JSM, then import it into browser windows at startup: https://hg.mozilla.org/try/rev/52e3ff20f1a7128bf1432a770ea434af4490deaf. The pro to this vs manually creating the DOM on popupshowing is that we could still use DTDs while waiting for Fluent conversion (although you could also use them if you used MozXULElement.parseXULToFragment). The con is that you still need to pretty eagerly add the DOM instead of waiting for popupshowing.

At one point I tried extracting the entire browser.xul contents inside of <window> except for the script tags into a shared frag. IIRC this helped with tpaint (since the frag was already cached in memory and importing it into the new document is faster), but I believe there were some issues the the menubar not working properly on Windows. That may have been something specific with browser.xhtml though.

2. DevTools has a JS API for menus at https://searchfox.org/mozilla-central/source/devtools/client/framework/menu.js, that I've thought about porting to toolkit if we want to use it. This lets you hook into the contextmenu event and then set up the menu state from JS at that time. Porting our browser window menus to use this is certainly more work than somehow creating the existing DOM when the popup is opened. Though FWIW when we've done this for devtools windows it's helped to make the menus easier to test and understand since you can look in a single function for exactly what the state will be (rather than it being tracked in external code which is setting [disabled], [hidden], etc on menuitems out of band).

Comment 3

[Brian Grinstead [:bgrins]]

Also, here's a document discussing potential replacement for XUL menu elements that might be relevant: https://docs.google.com/document/d/1mzJhy0pzvzK3PGCgsdQAqOzq-HYbpvnclD1Raoy3M6U/edit.

Comment 4

[:Gijs (he/him)]

We did a bunch of this work in bug 1624683. The most recent update is probably in bug 1515799 comment 16 . I also have vague recollections that I tried lazy-loading all of the main menubar but that did not produce much improvement.

Any work here should probably start by (a) trying to stuff most of the remaining non-templated contents of #mainPopupSet into template tags, and (b) assessing whether that impacts any startup or window opening tests (./mach try perf can help with this), and if so, (c) bisecting that (or profiling it with the firefox profiler!) to see which specific menus are the most bang-for-buck.

One complication with this is that there is a decent amount of code in our codebase that will try to getElementById stuff in those menus, and that code will start failing once the markup is no longer in the DOM. As a result, any attempt to do this will need to somehow compensate for this. Although commenting it out or removing it or whatever will obviously "fix" this (and for a bunch of it, if we were doing this "for real", we could delay running such code until after we insert the markup and fire popupshowing or similar), it's possible that for some of it there is unavoidable cost, and so a naive trypush could suggest we could avoid that cost, when in practice we cannot do so easily.

It's possible some of the other avenues in the linked comment (like making the bindings of our common elements like toolbarbutton lighter by removing markup and logic we don't use in practice) is a quicker way of gaining something here. Again, trypushes and/or measuring locally are probably the best way of figuring that out.