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)
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().
Comment 1•11 years ago
|
||
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.
Comment 3•11 years ago
|
||
B2G uses E10S so I don't think we can skip this there. Android probably doesn't usually have so many unrestored tabs.
Comment 4•11 years ago
|
||
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.
Comment 5•11 years ago
|
||
Note, we do cache the compiled frame scripts and just execute the compiled version each time such is loaded.
Comment 6•11 years ago
|
||
> 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
Comment 7•11 years ago
|
||
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.
Updated•11 years ago
|
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.
Comment 9•8 years ago
|
||
(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
Comment 10•8 years ago
|
||
(I assume we'll move more the internal stuff to C++ + IPDL, but that ofc isn't available to addons)
Comment 11•8 years ago
|
||
(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.
Comment 12•8 years ago
|
||
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
Comment 13•8 years ago
|
||
> │ │ ├──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.
Comment 14•8 years ago
|
||
(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)
Comment 15•8 years ago
|
||
Bug 1295967 should reduce the memory used for shapes.
Comment 16•8 years ago
|
||
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.
Comment 17•8 years ago
|
||
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.
Comment 18•8 years ago
|
||
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++.
Comment 19•8 years ago
|
||
(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.
Comment 20•8 years ago
|
||
filed bug 1299505
Comment 21•7 years ago
|
||
(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
Updated•2 years ago
|
Severity: normal → S3
Comment 22•1 year ago
|
||
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)
Comment 23•1 year ago
|
||
Hi :smaug, can you help to triage this bug? Thanks!
Flags: needinfo?(jstutte) → needinfo?(smaug)
Comment 24•1 year ago
|
||
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.
Description
•