993282 - Lazy load more js modules

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Runtime, defect)

Description

[[:fabrice] Fabrice Desré]

Attachment: modules-list.zip

We can do better than the current state.
Here's the list of modules loaded in various processes before and after these patches.

I get around 300kb of USS reduction in the nuwa and preallocated processes.

Comment 1

[[:fabrice] Fabrice Desré]

Attachment: less-modules.patch

part 1

Comment 2

[[:fabrice] Fabrice Desré]

Attachment: even-less-modules.patch

part 2

Comment 3

[[:fabrice] Fabrice Desré]

Attachment: lazy-ril.patch

part 3

Comment 4

[[:fabrice] Fabrice Desré]

Comment on attachment 8403083 [details] [diff] [review]
less-modules.patch

Review of attachment 8403083 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Sid, ignore the debugging change in XPCComponents.cpp

Comment 5

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8403086 [details] [diff] [review]
lazy-ril.patch

Review of attachment 8403086 [details] [diff] [review]:
-----------------------------------------------------------------

MobileMessageDB.jsm also imports ril_consts.js. We may do that as well in this bug.

::: dom/system/gonk/RILContentHelper.js
@@ +20,5 @@
>  Cu.import("resource://gre/modules/DOMRequestHelper.jsm");
>  Cu.import("resource://gre/modules/Services.jsm");
>  Cu.import("resource://gre/modules/XPCOMUtils.jsm");
>  
> +XPCOMUtils.defineLazyGetter(this, "RIL", function () {

I think you can just use:

XPCOMUtils.defineLazyModuleGetter(this, "RIL",
  "resource://gre/modules/ril_consts.js");

@@ +455,5 @@
>    },
>  };
>  
>  function RILContentHelper() {
> +  dump("YYY Starting RILContentHelper()\n");

Just a reminder. Please remove before landing.

@@ +515,5 @@
>    updateDebugFlag: function updateDebugFlag() {
>      try {
> +      /*DEBUG = RIL.DEBUG_CONTENT_HELPER ||
> +              Services.prefs.getBoolPref(kPrefRilDebuggingEnabled);*/
> +        DEBUG = false;

Please remove this before landing.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +22,5 @@
>  Cu.import("resource://gre/modules/Sntp.jsm");
>  Cu.import("resource://gre/modules/systemlibs.js");
>  Cu.import("resource://gre/modules/Promise.jsm");
>  
> +XPCOMUtils.defineLazyGetter(this, "RIL", function () {

Ditto.

@@ +779,5 @@
>    mdn: null
>  };
>  
>  function RadioInterfaceLayer() {
> +  dump("YYY Starting RadioInterfaceLayer()\n");

Ditto.

::: dom/telephony/gonk/TelephonyProvider.js
@@ +10,5 @@
>  Cu.import("resource://gre/modules/XPCOMUtils.jsm");
>  Cu.import("resource://gre/modules/Services.jsm");
>  Cu.import("resource://gre/modules/Promise.jsm");
>  
> +XPCOMUtils.defineLazyGetter(this, "RIL", function () {

Ditto.

Comment 6

[Gene Lian (I already quit Mozilla)]

After some thoughts, we cannot use defineLazyModuleGetter because "RIL" is not a valid export in ril_consts.js. Please keep the original use of defineLazyGetter. Thanks!

Comment 7

[[:fabrice] Fabrice Desré]

Hm, not sure what happened but Emulator mochitests are not happy... https://tbpl.mozilla.org/?tree=Try&rev=bed3b401ab04

Comment 8

[James Zhang (Spreadtrum)]

(In reply to Fabrice Desré [:fabrice] from comment #7)
> Hm, not sure what happened but Emulator mochitests are not happy...
> https://tbpl.mozilla.org/?tree=Try&rev=bed3b401ab04

You can set [tarako only].

Comment 9

[James Zhang (Spreadtrum)]

Let's land it on v1.3t and save about 1MB memory.

Comment 10

[Steven Yang [:styang]]

Triage: 1.3T+

Comment 11

[[:fabrice] Fabrice Desré]

remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/96e1a542bb45
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/6b9c8d6ab401
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/30e0991d49e0

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back these out in https://hg.mozilla.org/integration/b2g-inbound/rev/7616388e078a for b2g emulator mochitest-3 failures: https://tbpl.mozilla.org/php/getParsedLog.php?id=39243929&tree=B2g-Inbound

Comment 13

[[:fabrice] Fabrice Desré]

Wes, I had a similar intermittent on try:
https://tbpl.mozilla.org/?tree=Try&rev=97995867b3c3

Do you mind restart the test?

Comment 14

[[:fabrice] Fabrice Desré]

I could not reproduce the failures locally so I pushed the 3 parts to try in succession:
 
Part 1:
https://tbpl.mozilla.org/?tree=Try&rev=b18479095aee

Parts 1 + 2:
https://tbpl.mozilla.org/?tree=Try&rev=325a4e34253f

Parts 1 + 2 + 3:
https://tbpl.mozilla.org/?tree=Try&rev=c716badf1e49

and of course, none of them fail in M3...

Comment 15

[[:fabrice] Fabrice Desré]

After discussion with RyanVM, re-landing:

remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/3dbd84a99ce8
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/13ca7eb38c28
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/254616e044b1

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3dbd84a99ce8
https://hg.mozilla.org/mozilla-central/rev/13ca7eb38c28
https://hg.mozilla.org/mozilla-central/rev/254616e044b1

Comment 17

[[:fabrice] Fabrice Desré]

remote:   https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/843e28374449
remote:   https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/3862553bdea6
remote:   https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/9c52f1e9a67d

Comment 18

[Ed Morley [:emorley]]

Backed out for causing bug 1008357:
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/cbf3e3160d8b
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/9265b52eda3f
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/548e11df4056

Comment 19

[Joe Cheng [:jcheng] (please needinfo)]

ni? Fabrice
Wonder if we are relanding this? thanks

Comment 20

[[:fabrice] Fabrice Desré]

(In reply to Joe Cheng [:jcheng] from comment #19)
> ni? Fabrice
> Wonder if we are relanding this? thanks

Not before the root cause of bug 1008357 is fixed. Note that we didn't backout on tarako.

Comment 21

[James Zhang (Spreadtrum)]

(In reply to Fabrice Desré [:fabrice] from comment #20)
> (In reply to Joe Cheng [:jcheng] from comment #19)
> > ni? Fabrice
> > Wonder if we are relanding this? thanks
> 
> Not before the root cause of bug 1008357 is fixed. Note that we didn't
> backout on tarako.

bug 1008357 is fixed.

Comment 22

[[:fabrice] Fabrice Desré]

(In reply to James Zhang from comment #21)
> (In reply to Fabrice Desré [:fabrice] from comment #20)
> > (In reply to Joe Cheng [:jcheng] from comment #19)
> > > ni? Fabrice
> > > Wonder if we are relanding this? thanks
> > 
> > Not before the root cause of bug 1008357 is fixed. Note that we didn't
> > backout on tarako.
> 
> bug 1008357 is fixed.

Well, the test failure was closed because we backed out this one. The underlying issue is still there.

Comment 23

[James Zhang (Spreadtrum)]

Do we fix bug 1005120.

Comment 24

[[:fabrice] Fabrice Desré]

(In reply to James Zhang from comment #23)
> Do we fix bug 1005120.

I'd love that, but this is a different issue.

I filed bug 1015027 to unblock this bug.

Comment 25

[Sheila Mooney]

Sprint 1 is finished so reassigning to the current sprint.

Comment 26

[Nicolas B. Pierron [:nbp]]

Attachment: Work around Bug 1015027

Comment 27

[Nicolas B. Pierron [:nbp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2a6184672f1e
https://hg.mozilla.org/integration/mozilla-inbound/rev/41fe339ca4ea
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9ca30286579
https://hg.mozilla.org/integration/mozilla-inbound/rev/b67f3001c73d

These patches were tested on the emulator-opt as part of the following Try push:
  https://tbpl.mozilla.org/?tree=Try&rev=ca579e21501a

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Still hitting bug 1008357. Backed out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9d16a614fcc

https://tbpl.mozilla.org/php/getParsedLog.php?id=40973434&tree=Mozilla-Inbound

Comment 29

[Gregor Wagner [:gwagner]]

Filed bug 1024157 for master.

Comment 30

[James Zhang (Spreadtrum)]

Is this patch land on v1.3t?

Comment 31

[Gregor Wagner [:gwagner]]

(In reply to James Zhang from comment #30)
> Is this patch land on v1.3t?

Yes it got landed according to comment 17. Comment 18 backed out the patch on inbound.

I also checked hg and it's in:
https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/log/174dfde44194/dom/contacts/fallback/ContactDB.jsm