Closed Bug 844661 Opened 11 years ago Closed 1 year ago

Frame scripts use 170+ KB per tab (inProcessTabChildGlobal in about:memory), even for the not-yet-restored tabs

Categories

(Core :: General, defect)

x86
macOS
defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: asqueella, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [MemShrink:P3][Snappy])

Firefox loads a few content scripts for each tab:
http://hg.mozilla.org/mozilla-central/annotate/0d0071a94fe9/browser/base/content/browser.js#l1148
http://hg.mozilla.org/mozilla-central/annotate/0d0071a94fe9/toolkit/mozapps/extensions/addonManager.js#l47
http://hg.mozilla.org/mozilla-central/annotate/0d0071a94fe9/toolkit/components/satchel/nsFormHistory.js#l93

The scripts are run even for the not-yet-restored tabs (created when restoring a session with "Don't load tabs until selected" preference checked), contributing (170+ KB per tab) to excessive memory usage being investigated in bug 681201.

in bug 681201 comment 73 Gavin says "I think it would be easy enough to eliminate that entirely now that we've dropped e10s support."

See http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsIMessageManager.idl for information about the message manager and content scripts.

STEPS TO REPRODUCE:
0. Ensure the "Don't load tabs until selected" preference is selected
1. open 100 tabs by running this from the Error Console:
> for (var i = 0; i<100; i++) { window.top.opener.gBrowser.addTab("about:blank") }
2. Restart
3. Open about:memory, then click "Minimize memory usage" a few times

Here are the measurements (under explicit/js-non-window/compartments/non-window-global):
	17.49 MB (53.82%) -- inProcessTabChildGlobal?ownedBy=chrome://browser/content/browser.xul
	├──13.95 MB (42.93%) -- gc-heap
	│  ├───4.57 MB (14.05%) ── unused-gc-things
	│  ├───3.63 MB (11.16%) ── objects/function
	│  ├───3.52 MB (10.83%) -- shapes
	│  │   ├──2.42 MB (07.45%) ── tree/global-parented
	│  │   └──1.10 MB (03.39%) ── base
	│  └───2.24 MB (06.88%) ── sundries
	├───1.79 MB (05.52%) ── shapes-extra/compartment-tables
	├───0.97 MB (02.99%) ── objects-extra/slots
	└───0.78 MB (02.39%) ── other-sundries

Commenting out all the messageManager.loadFrameScript calls listed above makes the inProcessTabChildGlobal measurement disappear.

Uncommenting any one of them makes the inProcessTabChildGlobal measurement jump to 15+ MB.

The nsInProcessTabChildGlobal initialization (and presumably the JS execution) is called from nsXULElement::BindToTree for the <browser> element, see http://hg.mozilla.org/mozilla-central/annotate/0d0071a94fe9/content/xul/content/src/nsXULElement.cpp#l735 and nsXULElement::LoadSrc().
Blocks: 681201
This will probably improve startup times.
Blocks: 447581
Whiteboard: [MemShrink] → [MemShrink][Snappy]
Forgive me if this is a dumb or non-contributive question, but does this affect us on non-desktop platforms as well? (aka, mobile)  I know that every bit counts on phones.
(If so, guessing the Platform fields can be updated.)

P.S. This is very exciting bug for those of us who keep many tabs open as this may make a noticeable difference in startup and memory usage.
B2G uses E10S so I don't think we can skip this there.  Android probably doesn't usually have so many unrestored tabs.
Yeah, I think the potential improvement in start-up times (something glandium complained about in the many-tabs case) is more interesting here than the memory reduction.
Note, we do cache the compiled frame scripts and just execute the compiled version each time
such is loaded.
> Uncommenting any one of them makes the inProcessTabChildGlobal measurement jump to 15+ MB.

Given this comment it seems that this memory isn't necessarily dependent on the the scripts' code but more likely to be a fixed cost from inProcessTabChildGlobal
Well, ofc we need to create the JS objects for each tabchildglobal separately.
It just shouldn't be the script itself taking memory, but the objects the script create.
Whiteboard: [MemShrink][Snappy] → [MemShrink:P3][Snappy]
Many addons use frame scripts, but all the frame scripts (built-ins and addons) run in the same zone due to the shared-global thingy. It makes it difficult attribute the memory usage to any individual script.

Would it make sense to have one zone per registered frame script? 

(In reply to Olli Pettay [:smaug] (vacation Aug 25-28) from comment #7)
> It just shouldn't be the script itself taking memory, but the objects the
> script create.

An approach I have used in several addons is to to load an IIFE as frame script which imports a JSM and passes the tabchildglobal to the JSM. That way the JS prototypes and listener functions only have to be instantiated once and can be reused and the IIFE can be GCed immediately (right?).

Another option might be process script + observer notifications for new top level windows / enumerating frames, but that's far more cumbersome, even though it should have been the supported way from the start.

Frame scripts just have ridiculous overhead once you got a few addons installed.
(In reply to The 8472 from comment #8)
> Many addons use frame scripts, but all the frame scripts (built-ins and
> addons) run in the same zone due to the shared-global thingy. It makes it
> difficult attribute the memory usage to any individual script.
> 
> Would it make sense to have one zone per registered frame script? 

I assume that would increase the overall memory usage.
And I don't even know whether it is in any possible to have one global but several zones. I doubt.

> 
> (In reply to Olli Pettay [:smaug] (vacation Aug 25-28) from comment #7)
> > It just shouldn't be the script itself taking memory, but the objects the
> > script create.
> 
> An approach I have used in several addons is to to load an IIFE as frame
> script which imports a JSM and passes the tabchildglobal to the JSM. That
> way the JS prototypes and listener functions only have to be instantiated
> once and can be reused and the IIFE can be GCed immediately (right?).
> 
> Another option might be process script + observer notifications for new top
> level windows / enumerating frames, but that's far more cumbersome, even
> though it should have been the supported way from the start.

Another option is to use process level message manager's script global.
Is that exposed to addons?


> 
> Frame scripts just have ridiculous overhead once you got a few addons
> installed.
Any data on this? Would be good to understand where the memory is spent
(I assume we'll move more the internal stuff to C++ + IPDL, but that ofc isn't available to addons)
(In reply to Olli Pettay [:smaug] (vacation Aug 25-28) from comment #9)
> (In reply to The 8472 from comment #8)
> > Would it make sense to have one zone per registered frame script? 
> 
> I assume that would increase the overall memory usage.
> And I don't even know whether it is in any possible to have one global but
> several zones. I doubt.

Hrm, yeah, it was more about attributing it to individual scripts in about:memory, right now everything gets lumped into a single entry.

> > (In reply to Olli Pettay [:smaug] (vacation Aug 25-28) from comment #7)
> > An approach I have used in several addons is to to load an IIFE as frame
> > script which imports a JSM and passes the tabchildglobal to the JSM. That
> > way the JS prototypes and listener functions only have to be instantiated
> > once and can be reused and the IIFE can be GCed immediately (right?).
> > 
> > Another option might be process script + observer notifications for new top
> > level windows / enumerating frames, but that's far more cumbersome, even
> > though it should have been the supported way from the start.
> 
> Another option is to use process level message manager's script global.
> Is that exposed to addons?

Isn't that what a process script does?

The part that makes using process scripts more cumbersome than frame scripts is accessing the frame script globals from the process script global.

In other words, it's more convenient to be wasteful and use frame scripts.


> > Frame scripts just have ridiculous overhead once you got a few addons
> > installed.
> Any data on this? Would be good to understand where the memory is spent

This is from a parent process with several addons using frame scripts, including one that uses the sdk/remote/child module which provides a low-level wrapper around frame scripts.

│  │    ├──945.68 MB (40.41%) -- zone(0x272274f2800)
│  │    │  ├──621.57 MB (26.56%) -- compartment([System Principal], inProcessTabChildGlobal?ownedBy=chrome://browser/content/browser.xul)
│  │    │  │  ├──386.95 MB (16.53%) -- classes
│  │    │  │  │  ├──155.69 MB (06.65%) -- shapes
│  │    │  │  │  │  ├──126.40 MB (05.40%) -- gc-heap
│  │    │  │  │  │  │  ├──108.85 MB (04.65%) ── tree [1517]
│  │    │  │  │  │  │  └───17.56 MB (00.75%) ++ (2 tiny)
│  │    │  │  │  │  └───29.28 MB (01.25%) ++ malloc-heap
│  │    │  │  │  ├──141.34 MB (06.04%) -- class(Function)/objects
│  │    │  │  │  │  ├──135.23 MB (05.78%) ── gc-heap [1517]
│  │    │  │  │  │  └────6.11 MB (00.26%) ── malloc-heap/slots [1517]
│  │    │  │  │  ├───59.10 MB (02.53%) -- class(<non-notable classes>)/objects
│  │    │  │  │  │   ├──32.50 MB (01.39%) ── gc-heap [1517]
│  │    │  │  │  │   └──26.60 MB (01.14%) -- malloc-heap
│  │    │  │  │  │      ├──25.09 MB (01.07%) ── slots [1517]
│  │    │  │  │  │      └───1.51 MB (00.06%) ++ (2 tiny)


├────362.88 MB (15.50%) -- window-objects
│    ├──285.98 MB (12.22%) ++ (1520 tiny)


Most of those windows are about:blanks, so frame script footprint is more than twice the size of the about:blank windows themselves.
Err, sorry, I didn't paste the complete frame script subtree:

│  │    ├──945.68 MB (40.41%) -- zone(0x272274f2800)
│  │    │  ├──621.57 MB (26.56%) -- compartment([System Principal], inProcessTabChildGlobal?ownedBy=chrome://browser/content/browser.xul)
│  │    │  │  ├──386.95 MB (16.53%) -- classes
│  │    │  │  │  ├──155.69 MB (06.65%) -- shapes
│  │    │  │  │  │  ├──126.40 MB (05.40%) -- gc-heap
│  │    │  │  │  │  │  ├──108.85 MB (04.65%) ── tree [1517]
│  │    │  │  │  │  │  └───17.56 MB (00.75%) ++ (2 tiny)
│  │    │  │  │  │  └───29.28 MB (01.25%) ++ malloc-heap
│  │    │  │  │  ├──141.34 MB (06.04%) -- class(Function)/objects
│  │    │  │  │  │  ├──135.23 MB (05.78%) ── gc-heap [1517]
│  │    │  │  │  │  └────6.11 MB (00.26%) ── malloc-heap/slots [1517]
│  │    │  │  │  ├───59.10 MB (02.53%) -- class(<non-notable classes>)/objects
│  │    │  │  │  │   ├──32.50 MB (01.39%) ── gc-heap [1517]
│  │    │  │  │  │   └──26.60 MB (01.14%) -- malloc-heap
│  │    │  │  │  │      ├──25.09 MB (01.07%) ── slots [1517]
│  │    │  │  │  │      └───1.51 MB (00.06%) ++ (2 tiny)
│  │    │  │  │  ├───30.69 MB (01.31%) ++ class(Object)/objects
│  │    │  │  │  └────0.13 MB (00.01%) ++ class(Array)/objects
│  │    │  │  ├──129.71 MB (05.54%) -- scripts
│  │    │  │  │  ├──101.74 MB (04.35%) ── gc-heap [1517]
│  │    │  │  │  └───27.96 MB (01.19%) ── malloc-heap/data [1514]
│  │    │  │  ├───29.81 MB (01.27%) ── compartment-tables [1517]
│  │    │  │  ├───27.44 MB (01.17%) ── private-data [1513]
│  │    │  │  ├───23.93 MB (01.02%) -- type-inference
│  │    │  │  │   ├──23.92 MB (01.02%) ── object-type-tables [1514]
│  │    │  │  │   └───0.01 MB (00.00%) ── type-scripts
> │  │    ├──945.68 MB (40.41%) -- zone(0x272274f2800)
> │  │    │  ├──621.57 MB (26.56%) -- compartment([System Principal],
> inProcessTabChildGlobal?ownedBy=chrome://browser/content/browser.xul)
> │  │    │  │  ├──386.95 MB (16.53%) -- classes
> │  │    │  │  │  ├──155.69 MB (06.65%) -- shapes
> │  │    │  │  │  │  ├──126.40 MB (05.40%) -- gc-heap
> │  │    │  │  │  │  │  ├──108.85 MB (04.65%) ── tree [1517]

An important thing there that may not be obvious is the "[1517]", which means that there are 1517 duplicate entries for that path. In other words, there are 1517 system compartments(!) and their trees are being combined in about:memory. I don't know how or why there are so many, but that's why memory usage is high here.
(In reply to The 8472 from comment #11)
> Isn't that what a process script does?
uh, yes, sorry.

> In other words, it's more convenient to be wasteful and use frame scripts.
right, but what if addon sdk hide the cumbersomeness?


> This is from a parent process with several addons using frame scripts,
> including one that uses the sdk/remote/child module which provides a
> low-level wrapper around frame scripts.
So this is non-e10s, right? How does this compare to the case when there are no addons?



> Most of those windows are about:blanks, so frame script footprint is more than twice the size of the
> about:blank windows themselves.
Well, I'd be quite worried if about:blank windows were any heavier.


I wonder if we're reporting the memory usage of scripts right, since TabChildGlobals should in general cache/reuse the scripts. (unless someone is silly enough to use data: urls)
Bug 1295967 should reduce the memory used for shapes.
I'm trying to nail down the difference between addons on / off. There actually doesn't seem to be much of a difference between in the inProcessTabChildGlobal footprint itself. But the system zone differs by about 100MB (856.94 MB -> 757.76 MB).

I can't exactly compare with the previously pasted values yet since it is somewhat affected by uptime. Bug 129567 may also have some impact, but the shapes (for the zone now) still range in 100+ MB, so it's does not seem to be a radical reduction.

(In reply to Olli Pettay [:smaug] (vacation Aug 25-28) from comment #14)
> So this is non-e10s, right?

That is with e10s on, but lazy tabs (about:blank) stay in the parent process until they're touched and do have frame scripts.


> I wonder if we're reporting the memory usage of scripts right, since
> TabChildGlobals should in general cache/reuse the scripts. (unless someone
> is silly enough to use data: urls)

There seems to be at least one:

> Services.mm.getDelayedFrameScripts()[6][0]
> 
> "data:,(function (contentFrame, PATH) {
>   let { registerContentFrame } = Components.utils.import(PATH + 'framescript/content.jsm', {});
>   registerContentFrame(contentFrame);
> })(this, "resource://gre/modules/commonjs/");"

Seems to be part of the addon SDK.

And who knows what else might be happening through the subscriptloader or eval.
Ok, i have to retract my earlier statement.

Memory attributed to frame script globals is fairly stable and the addons i'm using don't add much relative to what's already present in vanilla firefox. The system zone growth actually comes from JSMs and SDK modules.

That said, they still add up to hundreds of MB.


Tangential idea:

Would it be possible to defer/incrementalize the loading of frame scripts and message delivery on background about:blank tabs?
The idea is that those tabs don't do anything anyway and messaging from the parent process is asynchronous so it should be fine if anything expecting a request-response cycle doesn't hear back from the frames until the session is restored.

That wouldn't reduce footprint, but at least it would speed up startup.
Sounds reasonable. I don't know how we restore tabs normally (I've explicitly set the pref to load all the tabs asap). Could you file a new bug about that. That would be more Firefox bug than a core bug.

To reduce memory usage we should somehow make JS stuff more lightweight and/or convert more stuff to C++.
(In reply to Olli Pettay [:smaug] from comment #18)
> To reduce memory usage we should somehow make JS stuff more lightweight



comment #0 says:
> Uncommenting any one of them makes the inProcessTabChildGlobal measurement
> jump to 15+ MB.

That sounds like there is some initial/minimum footprint per frame global that's not due to allocations in the scripts.
filed bug 1299505
(In reply to The 8472 from comment #17)
> That wouldn't reduce footprint, but at least it would speed up startup.

For footprint, I'd say you could track/depend bug 1054660
Severity: normal → S3

The severity field for this bug is relatively low, S3. However, the bug has 22 votes.
:jstutte, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Flags: needinfo?(jstutte)

Hi :smaug, can you help to triage this bug? Thanks!

Flags: needinfo?(jstutte) → needinfo?(smaug)

Frame scripts are deprecated and we have very few use cases for them anymore. With Fission we got JS Actors.
Also, memory usage was optimized quite a bit for Fission.

So, I think this bug as such isn't too useful anymore. New bugs should be filed if there are similar issues with the current architecture.

Status: NEW → RESOLVED
Closed: 1 year ago
Flags: needinfo?(smaug)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.