Closed Bug 1479309 Opened 2 years ago Closed 2 years ago

Don't load shield-content-frame.js until needed

Categories

(Firefox :: Normandy Client, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: kmag, Assigned: kmag)

References

(Blocks 1 open bug)

Details

(Whiteboard: [overhead:7k])

Attachments

(1 file)

shield-content-frame.js is only needed in content processes when an event received from a shield page. It's only ever really needed when about:studies has been loaded in a process, but the quickest path to getting rid of it it is just loading it when we get a "ShieldPageEvent" event in a frame script.
Comment on attachment 8995828 [details]
Bug 1479309: Don't load shield-content-frame.js until needed.

https://reviewboard.mozilla.org/r/260166/#review267386

Thanks!

Also, since this is blocking the memshrink bug, I'm curious if you've profiled how much memory these changes save? If you haven't already profiled it, don't worry about. I'm sure the the changes are worth it, I'm just curious.
Attachment #8995828 - Flags: review?(mcooper) → review+
(In reply to Michael Cooper [:mythmon] from comment #3)
> Also, since this is blocking the memshrink bug, I'm curious if you've
> profiled how much memory these changes save? If you haven't already profiled
> it, don't worry about. I'm sure the the changes are worth it, I'm just
> curious.

See whiteboard. According to dominator tree analysis, this frame script uses about 7K in a process with a single tab (more for processes with multiple tabs). In practice, actual savings tend to be slightly more than that tool reports, though.
Component: General → Normandy Client
Product: Shield → Firefox
https://hg.mozilla.org/mozilla-central/rev/2d46e1fe3121
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Backout by csabou@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/fe6020e5c9d9
Backed out 12 changesets (bug 1479309, bug 1479312, bug 1479313, bug 1479310, bug 1479235, bug 1479945, bug 1479241, bug 1479318) for causing a big performance regression on OS X. a=backout
Backed out for causing a big performance regression on OS X and as request by igoldan. 

https://hg.mozilla.org/mozilla-central/rev/fe6020e5c9d901a40fa2e7ea2f1ab2a36bf6d856
Status: RESOLVED → REOPENED
Flags: needinfo?(kmaglione+bmo)
Resolution: FIXED → ---
Target Milestone: Firefox 63 → ---
https://hg.mozilla.org/mozilla-central/rev/a396e7027858
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Flags: needinfo?(kmaglione+bmo)
You need to log in before you can comment on or make changes to this bug.