Temporarily merge JSMs part 2: electric boogaloo

RESOLVED FIXED in Firefox 21

Status

Firefox Health Report
Client: Desktop
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 21
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 707985 [details] [diff] [review]
Merge more JSMs, v1

Merge moar JSMs because of compartment clownshoes.

Savings:

async.js - 81k
observers.js - 93k
rest.js - 106k

Increases:

datareporting - 205 -> 236 (31)
healthreport - 462 -> 533 (71)

Net: 178kb saved.

After this patch, FHR loads the following JSMs that aren't in one of the two main compartments:

* Sqlite.jsm
* Task.jsm
* services-common/utils.js
* services-common/preferences.js
* services-common/log4moz.js

Sqlite.jsm and Task.jsm aren't worth merging IMO (more users and not in /services).

log4moz.js and utils.js are loaded by Sqlite.jsm. So, we'd have to hack up Sqlite.jsm. I believe this is out of the question.

log4moz.js has some singletons and having an instance in multiple compartments might make things act weird.

preferences.js is loaded by both of FHR's main compartments. We probably could load it in both without any issue. But, I think it's easier to not merge.

As rnewman stated in the previous bug, this merging does scare me to some degree. I'm not overly worried (I trust our tests). But, it's extremely hacky and I want to see it gone immediately.

All told, FHR is hovering around 1.4MB with this patch applied. That counts log4moz (176), prefs (133), utils (131), and sqlite (147) (which are shared among multiple features). SQLite is likely the only feature that is always running, so it takes the hit for increasing memory. We are around 740k for the 2 main compartments and 80k for the SQLite connection.

In bug 836120, I freed about ~628kb of memory from Sync when Sync isn't configured by not loading modules. However, some of those modules are also used by FHR (like utils and log4moz). So, the net gain isn't as high as I would like. Still, Sync's code is better and we do still gain a little more from making Sync behave better.
Attachment #707985 - Flags: review?(rnewman)
Comment on attachment 707985 [details] [diff] [review]
Merge more JSMs, v1

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

Task.jsm is 72 non-comment lines of simple code; I don't think it falls in the same bucket as Sqlite.jsm.

I wholeheartedly support preprocessing that into the mix, too, and seeing what the outcome is. If it's a net win, do it.
Attachment #707985 - Flags: review?(rnewman) → review+
(Assignee)

Comment 2

5 years ago
https://hg.mozilla.org/services/services-central/rev/34a80b4e0acd
Assignee: nobody → gps
Status: NEW → ASSIGNED
Whiteboard: [MemShrink][fixed in services]
(Assignee)

Updated

5 years ago
Blocks: 836285
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/mozilla-central/rev/34a80b4e0acd
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [MemShrink][fixed in services] → [MemShrink]
Target Milestone: --- → mozilla21
(Assignee)

Updated

5 years ago
Blocks: 838461
Blocks: 831397

Updated

4 years ago
Component: Metrics and Firefox Health Report → Client: Desktop
Product: Mozilla Services → Firefox Health Report
Target Milestone: mozilla21 → Firefox 21
You need to log in before you can comment on or make changes to this bug.