807104 - Turn on jsloader.reuseGlobal in Fennec

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Bug 798491 added a jsloader.reuseGlobal pref which makes all jsm files share a global object, resulting in significant memory savings. We may want to turn this on in Fennec, particularly on low-memory devices.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Before we turn this on for real in Fennec we need to distinguish between platform JS (stuff from the omnijar) and extension JS (everything else) in mozJSComponentLoader.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Before/after about:memory dumps to see memory savings

FWIW I ran my AWSY-like benchmark (described in bug 792165) that opens the browser, loads a bunch of tabs, and then closes them on builds where this pref was turned on and off. The base cset I used was http://hg.mozilla.org/integration/mozilla-inbound/rev/74e7f7678c88 and then I had a try build of that cset + pref turned on.

Here is the resident memory usage info:

Memory snapshot    |      Base | Preffed on
-------------------------------------------
Start              |  57126912 |  53694464
StartSettled       |  65110016 |  58449920
TabsOpen           | 152129536 | 147804160
TabsOpenSettled    | 149647360 | 146546688
TabsOpenForceGC    | 147107840 | 143364096
TabsClosed         | 135135232 | 131588096
TabsClosedSettled  |  77430784 |  72970240
TabsClosedForceGC  |  77873152 |  71671808

So the resident memory usage savings seem to be on the order of 4-6 MB. I'm attaching the full about:memory dumps that go with each of these numbers in case anybody wants to look at it in more detail.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #1)
> Before we turn this on for real in Fennec we need to distinguish between
> platform JS (stuff from the omnijar) and extension JS (everything else) in
> mozJSComponentLoader.

Would you be willing to do that? :)

Alternatively, could you describe what needs to be done so that I or somebody else can take a crack at it?

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

It's not obvious to me how to do that.  I'll think about it.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Discussed this with khuey on IRC today:

khuey: I think what we want to do is just find out if the URI we're given is in the omnijar
khuey: I have no idea how to do that though :-)
kats: hmm can we just do a substring match on the URI?
khuey: well, omni.jar isn't in the URI ;-)
khuey: we need to ask the chrome registry or something
khuey: I've been meaning to ask bsmedberg about it

:bsmedberg, do you have any suggestions on the best way to do this?

Comment 6

[Benjamin Smedberg]

What are the URLs that you do have? Rather than asking "is this in the omnijar" (which is a hard question to answer), can we maybe just whitelist a set of chrome packages or resource mappings which should be globalified?

Comment 7

[Kartikaya Gupta (email:[e-mail])]

If I understand your question correctly (and if I looked in the right place in the code) the URLs look like this:

jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/DirectoryProvider.js
resource://gre/modules/XPCOMUtils.jsm
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/BrowserElementParent.js
resource://gre/modules/Services.jsm
resource://gre/modules/BrowserElementPromptService.jsm
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/SessionStore.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/addonManager.js
resource://gre/modules/AddonManager.jsm
resource://gre/modules/XPIProvider.jsm
resource://gre/modules/LightweightThemeManager.jsm
resource://gre/modules/PluginProvider.jsm
resource://gre/modules/AddonLogging.jsm
resource://gre/modules/FileUtils.jsm
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/TelemetryPing.js
resource://gre/modules/NetUtil.jsm
resource://gre/modules/ctypes.jsm
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/messageWakeupService.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/nsUpdateTimerManager.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/jsconsole-clhandler.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/BrowserCLH.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/nsDefaultCLH.js
resource://gre/modules/JNI.jsm
resource://gre/modules/accessibility/AccessFu.jsm
resource://gre/modules/accessibility/Utils.jsm
resource://gre/modules/PrivateBrowsingUtils.jsm
chrome://browser/content/InputWidgetHelper.js
chrome://browser/content/SelectHelper.js
resource://gre/modules/LightweightThemeConsumer.jsm
resource://gre/modules/Webapps.jsm
resource://gre/modules/ActivitiesService.jsm
resource://gre/modules/IndexedDBHelper.jsm
resource://gre/modules/AppsUtils.jsm
resource://gre/modules/PermissionsInstaller.jsm
resource://gre/modules/PermissionSettings.jsm
resource://gre/modules/OfflineCacheInstaller.jsm
resource://gre/modules/SystemMessagePermissionsChecker.jsm
resource://gre/modules/CertUtils.jsm
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/nsLoginManager.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/nsFormHistory.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/AboutRedirector.js
resource://gre/modules/SafeBrowsing.jsm
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/nsURLFormatter.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/nsUrlClassifierListManager.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/nsUrlClassifierLib.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/nsUrlClassifierHashCompleter.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/nsHandlerService.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/SiteSpecificUserAgent.js
resource://gre/modules/UserAgentOverrides.jsm
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/PromptService.js
jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/components/storage-mozStorage.js

Comment 8

[Benjamin Smedberg]

The ones "resource://gre/..." and "chrome://browser" you can just whitelist. The ones that start "jar:jar:file:///data/app/org.mozilla.fennec_kats-2.apk!/omni.ja!/..." are components in the omnijar, and for those a substring match should probably work just fine, since omni.ja is in the URI directly.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

I took at the relevant code in order to maybe try and implement this, but I'm a little unclear about some parts of the code. Specifically this block of code: http://hg.mozilla.org/mozilla-central/file/27a1c1839d42/js/xpconnect/loader/mozJSComponentLoader.cpp#l605

My question: is the mReuseLoaderGlobal guard around that block really needed?

If I understand correctly, it should return the correct object regardless of whether mReuseLoaderGlobal is true. Is that not the case?

Comment 10

[Mike Hommey [:glandium]]

Note that the PathifyURI function in startupcache/StartupCacheUtils.cpp has code that can be leveraged to answer the question 'is it in an omnijar'. That being said, running a build off dist/bin doesn't use omnijars.

Comment 11

[Mike Hommey [:glandium]]

Also note that addons can add overrides and resource://gre/ and chrome://browser may well point to resources that are *not* part of Firefox.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #9)
> I took at the relevant code in order to maybe try and implement this, but
> I'm a little unclear about some parts of the code. Specifically this block
> of code:
> http://hg.mozilla.org/mozilla-central/file/27a1c1839d42/js/xpconnect/loader/
> mozJSComponentLoader.cpp#l605
> 
> My question: is the mReuseLoaderGlobal guard around that block really needed?
> 
> If I understand correctly, it should return the correct object regardless of
> whether mReuseLoaderGlobal is true. Is that not the case?

I believe it is effectively a performance optimization.  Walking the function enclosure chain and doing the hashtable lookup will produce NULL when global reuse is not turned on, but it is slow.

Comment 13

[Nicholas Nethercote [inactive]]

FWIW:  when zones get finished (bug 759585) this shouldn't be necessary.

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Wontfixing based on the same argument as bug 835886.