Closed Bug 1381594 Opened 7 years ago Closed 7 years ago

Investigate the impact of applyCustomizations() on startup perf/jank

Categories

(Firefox :: Distributions, enhancement, P1)

56 Branch
enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: Felipe, Assigned: Felipe)

References

(Blocks 1 open bug)

Details

(Keywords: perf, stale-bug, Whiteboard: [reserve-photon-performance])

applyCustomizations() runs some potentially heavy code that is not usually seen in profiles because developers don't use it, but it may affect many users in the wild.

For example, I know that it does a resetDefaultPrefs() which implies in main-thread IO (_I think_, to be verified.. just giving an example)

I don't think we've ever looked into this closely in the past, so now with the Photon work it might be a good chance to do so.
The resetDefaultPrefs as always been a hack. We really need a proper fix for when distribution.js gets loaded.

See bug 947838.

Note that whatever we change (perhaps removing some distribution stuff if distribution isn't present) still needs to send the right notifications (distribution-customization-complete, etc.). Those are used for other startup reasons (although maybe not as important with WebExtensions).
Cool, thanks for the pointer. Note that my main goal here is not to do less work if distribution.ini is not present.. The intention is to see what can be improved perf-wise for the case of when it _is_ present.
Hi Florian, should bug 1381594 be part of the Performance MVP?
Flags: needinfo?(florian)
(In reply to :Felipe Gomes (needinfo me!) from comment #2)
> do less work if distribution.ini is not present..

That would be nice too though ;).

(In reply to Marco Mucci [:MarcoM] from comment #3)
> Hi Florian, should bug 1381594 be part of the Performance MVP?

Yes, although it's still only investigation at this point.
Flags: needinfo?(florian)
(In reply to Florian Quèze [:florian] [:flo] from comment #4)
> (In reply to :Felipe Gomes (needinfo me!) from comment #2)
> > do less work if distribution.ini is not present..
> 
> That would be nice too though ;).
> 
> (In reply to Marco Mucci [:MarcoM] from comment #3)
> > Hi Florian, should bug 1381594 be part of the Performance MVP?
> 
> Yes, although it's still only investigation at this point.

OK, it can be left out until actionable follow-up bugs are filed.
Whiteboard: [photon-performance]
Whiteboard: [photon-performance] → [photon-performance] [triage]
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-performance] [triage] → [photon-performance]
Could someone please add "perf" key word? Thanks.
Keywords: perf
Depends on: 1389344
Whiteboard: [photon-performance] → [reserve-photon-performance]
Flags: qe-verify? → qe-verify-
At the time that I took this bug, I focused mainly on bug 1389344, which affects every user, and not only users of distributions. Then I talked with Mike Kaply and did some bugzilla searches and there is already enough known and open bugs about possible improvements on this area.

The main bug that would improve things a lot is the double loading of prefs, which is tracked by bug 947838. A related bug to it is bug 802022.

Another bug is bug 801613, which is similar to the one fixed (bug 1389344). The difference is that bug 1389344 only solved the problem for users who don't have that file. However, if that file does exist, main-thread-io will still happen to read the contents of that file, and bug 801613 is about fixing that.

Solving these two bugs would probably be of great benefit to users of customized versions. There's some upcoming work on this area so maybe we get to fix or at least improve these while working at it.

I believe this bug has outlived its usefulness, so I'll close it with this summary.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Depends on: 947838, 802022, 801613
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.