Closed Bug 993282 Opened 6 years ago Closed 5 years ago

Lazy load more js modules

Categories

(Firefox OS Graveyard :: Runtime, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.3T+, b2g-v1.3T fixed)

RESOLVED FIXED
2.0 S3 (6june)
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3T --- fixed

People

(Reporter: fabrice, Assigned: fabrice)

References

Details

Attachments

(5 files)

Attached file 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.
part 1
part 2
Attached patch lazy-ril.patchSplinter Review
part 3
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → All
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
Attachment #8403083 - Flags: review?(psiddh)
Attachment #8403084 - Flags: review?(anygregor)
Attachment #8403086 - Flags: review?(vyang)
Attachment #8403084 - Flags: review?(anygregor) → review+
Attachment #8403083 - Flags: review?(psiddh) → review+
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.
Attachment #8403086 - Flags: review?(vyang) → review+
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!
Hm, not sure what happened but Emulator mochitests are not happy... https://tbpl.mozilla.org/?tree=Try&rev=bed3b401ab04
Depends on: 1005120
(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].
Let's land it on v1.3t and save about 1MB memory.
blocking-b2g: --- → 1.3T?
Flags: needinfo?(fabrice)
Assignee: nobody → fabrice
Flags: needinfo?(fabrice)
Triage: 1.3T+
blocking-b2g: 1.3T? → 1.3T+
Wes, I had a similar intermittent on try:
https://tbpl.mozilla.org/?tree=Try&rev=97995867b3c3

Do you mind restart the test?
Flags: needinfo?(fabrice)
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...
Depends on: 1008357
ni? Fabrice
Wonder if we are relanding this? thanks
Flags: needinfo?(fabrice)
(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.
Flags: needinfo?(fabrice)
(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.
(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.
Depends on: 1015027
(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.
Sprint 1 is finished so reassigning to the current sprint.
Target Milestone: 2.0 S1 (9may) → 2.0 S3 (6june)
Attachment #8433392 - Flags: review?(anygregor)
Attachment #8433392 - Flags: review?(anygregor) → review+
Blocks: 1024157
Filed bug 1024157 for master.
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Is this patch land on v1.3t?
(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
You need to log in before you can comment on or make changes to this bug.