Add-on manager sometimes blocks startup on blocklist checks, leading to slow startups
Categories
(Toolkit :: Add-ons Manager, defect, P2)
Tracking
()
People
(Reporter: Gijs, Assigned: aswan)
References
Details
(Whiteboard: [fxperf:p2])
Attachments
(1 file, 1 obsolete file)
Florian got this profile out of a try build earlier today: https://bit.ly/2UB7vzW
This is a profile captured on try for the browser_startup_mainthreadio.js test.
It seems we hit syncLoadManifest
and then end up blocking startup on loading the blocklist, which means we block on loading the dump through a worker with a bunch of IPC overhead etc.
It's not clear exactly what trips this, though the suspicion is it's the sideloaded mochitest add-ons. I don't really understand why we wouldn't hit this for builtin or system add-ons, and/or if this would affect people with crapw... I mean important security software that sideloads things, or something.
Either way, ideally we shouldn't need to do this. blocklist v3 will improve things here, but it'll still not be a good idea to block on things because we'll still need to load the data for the blocklist off disk, and even with the new infrastructure that's non-trivial IO - doing it away from the mainthread (as RS does here) doesn't help if we block the main thread on the IO's completion...
:aswan said he was interested in taking this.
Assignee | ||
Comment 1•5 years ago
|
||
(In reply to :Gijs (he/him) from comment #0)
It's not clear exactly what trips this, though the suspicion is it's the sideloaded mochitest add-ons. I don't really understand why we wouldn't hit this for builtin or system add-ons, and/or if this would affect people with crapw... I mean important security software that sideloads things, or something.
I'm pretty sure we do hit this for builtin/system addons (at least if I stick a dump() statement inside ExtensionBlocklistRS.ensureInitialized, I see it very early during startup in a new profile).
For sideloads (outside of testing), this is much more of a corner case now that we don't handle new sideloads (bug 1602839)
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
We don't put system addons on the blocklist and we don't sync them, so
there's no need to compute blocklist state or a sync guid for them. The
immediate motivation for this is to avoid initializing the blocklist
early in startup for the initial startup in a new profile or after a
browser upgrade when we're installing/udpating system and builtin addons.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 3•5 years ago
|
||
Comment 4•5 years ago
|
||
Comment on attachment 9138114 [details]
Bug 1625181 Skip blocklist checks for test extensions
Revision D69604 was moved to bug 1627336. Setting attachment 9138114 [details] to obsolete.
Assignee | ||
Comment 5•5 years ago
|
||
Spun off the special-powers and mochitest handling in bug 1627336
Comment 7•5 years ago
|
||
bugherder |
Comment 9•5 years ago
|
||
Backout merged: https://hg.mozilla.org/mozilla-central/rev/f509247a91c9
Updated•5 years ago
|
Comment 10•5 years ago
|
||
Comment 11•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Comment 12•5 years ago
|
||
== Change summary for alert #25614 (as of Tue, 14 Apr 2020 04:07:19 GMT) ==
Improvements:
4% JS windows7-32-shippable opt 79,428,166.00 -> 76,347,768.84
4% JS windows7-32-shippable opt 79,488,059.57 -> 76,528,361.21
3% JS linux1804-64-shippable opt 106,011,801.24 -> 102,683,548.74
3% JS windows10-64-shippable opt 108,224,629.82 -> 104,876,607.98
3% JS linux1804-64-shippable-qr opt 106,485,300.74 -> 103,221,107.36
3% JS macosx1014-64-shippable opt 107,583,008.37 -> 104,711,332.15
2% Heap Unclassified windows10-64-shippable opt 51,926,572.41 -> 50,696,683.11
2% JS windows10-64-shippable-qr opt 107,769,884.68 -> 105,335,162.11
2% Explicit Memory windows7-32-shippable opt 280,596,308.09 -> 274,870,133.10
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=25614
Reporter | ||
Comment 13•5 years ago
|
||
I think we now still do this work when a blocklist update comes in, because we ask the add-on manager for a list of all installed add-ons which (I think) includes these ones. Should we avoid updating blocklist states there, too?
Assignee | ||
Comment 14•5 years ago
|
||
(In reply to :Gijs (he/him) from comment #13)
I think we now still do this work when a blocklist update comes in, because we ask the add-on manager for a list of all installed add-ons which (I think) includes these ones. Should we avoid updating blocklist states there, too?
Yes, that's a good idea, filed bug 1633396
Description
•