986503 - Merge CustomizableUI.jsm and CustomizableWidgets.jsm to reduce compartment overhead

Status: RESOLVED WONTFIX

(Firefox :: Toolbars and Customization, defect)

Description

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

In bug 860642, we split out the widget definitions from CustomizableUI.jsm into CustomizableWidgets.jsm in an effort to reduce the complexity of CustomizableUI.jsm.

The JSM route was chosen arbitrarily by myself[1]. Maybe we should try doing a preprocessing include instead?

According to https://groups.google.com/forum/#!topic/mozilla.dev.platform/sAuhWQOAWic, CustomizableUI and CustomizableWidgets are new compartments contributing to the creeping start-up memory usage.

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=860642#c2

Comment 1

[Eric Rahm [:erahm]]

Mike, what do you mean by preprocessing include here? As in just |#include "CustomizableWidgets.jsm"|? I'm happy to try it out and see if there's any memory improvement.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Yeah, that would be sufficient. Though the final patch we may want to rename from CustomizableWidgets.jsm to CustomizableWidgets.inc.jsm to make it clear that the file is included, like we do for tabs.inc.css, etc.

Keeping .jsm at the end helps with syntax highlighting by editors.

Comment 3

[Matthew N. [:MattN]]

The other option is loadSubScript which has the advantage of not needing a build step on OSs which use symlinks for the object directory. This means you can simply restart the browser after editing without needing to build.

Comment 4

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

Hey arai, am I right in recalling that all privileged JSMs (and ESMs) within the same process are loaded in the same memory compartment these days, and that there'd be not much in the way of memory savings to merge the two JSMs mentioned in this bug?

Comment 5

[Tooru Fujisawa [:arai]]

yes, JSMs and system ESMs share the single global and they're in single compartment,
and merging 2 JSMs doesn't contribute to memory saving around that.

There's still slight overhead, such like the following, but at least memory consumption part is negligible compared to global/compartment which had many duplication.

• runtime memory overhead
  • per-JSM global this object, for storing global variables
  • script source's properties (js::ScriptSource and js::ScriptSourceObject)
  • top-level script's properties (js::BaseScript)
• load performance (both raw JSM file and cache)
  • string (including identifiers) atomization is performed for each file (atom table lookup is performed multiple times if 2 files have same strings)
• cache (serialization) filesize and encode/decode
  • strings (including identifiers) are encoded for each file, unless they're well-known names or 1-2 length

#include way can avoid all of them.
loadSubScript can avoid the per-JSM global this, but not others.

anyway, I think it's better using simple way as much as possible, and keep using multiple JSMs (and ESMs) would be the way to go,
unless we bump into other perf/memory issue with above or something else.

Comment 6

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

Okay, closing this out as a WONTFIX then. Thanks, arai!