1391792 - [meta] Reduce extension and plugin blocklist entries to improve Firefox startup time

Status: RESOLVED FIXED

(Toolkit :: Blocklist Implementation, enhancement, P3)

Description

[Jorge Villalobos [:jorgev] (he/him)]

The blocklist is loaded at startup and can cause some performance problems. We can try to improve this by removing obsolete entries and merging the many plugin blocks we have accumulated along the years.

Comment 1

[Jorge Villalobos [:jorgev] (he/him)]

One way to reduce load and parse times may be to remove the "details" node in the JSON files served by Kinto. That's metadata that Firefox doesn't need. Just doing that reduces my blocklist-addons.json from 450Kb to 230Kb and blocklist-plugins.json from 193Kb to 103Kb.

Comment 2

[Jorge Villalobos [:jorgev] (he/him)]

Out of ~650 add-on GUIDs in the add-on blocklist, only 120 are registered on AMO. The other ones were probably never signed, so they can't be installed in Firefox since version 48. We could remove all of them, or at least the older ones.

Comment 3

[Jorge Villalobos [:jorgev] (he/him)]

There are ~170 plugin blocks, which could probably be merged into ~50, since most are targeting specific version ranges of a single plugin (0->1.0, 1.0 -> 2.0, 2.0 -> 3.0, etc.) and can be collapsed into single blocks per plugin.

Comment 4

[Wayne Mery (:wsmwk)]

(In reply to Jorge Villalobos [:jorgev] from comment #2)
> Out of ~650 add-on GUIDs in the add-on blocklist, only 120 are registered on
> AMO. The other ones were probably never signed, so they can't be installed
> in Firefox since version 48. We could remove all of them, or at least the
> older ones.

Can we determine whether any eliminated blocks impact Thunderbird?  (we allow unsigned addons)
Or, would the blocks have only been valid for Firefox?

Comment 5

[Jorge Villalobos [:jorgev] (he/him)]

The majority of add-ons we've blocked along the years have only affected Firefox and wouldn't make sense in Thunderbird. Even if they related to Thunderbird, most of these add-ons were blocked many years ago and probably don't exist anymore.

At the moment I'm focusing on plugin blocks because they have very little risk and have the biggest wins. When I look into add-on blocks I'll be more careful about what is taken out.

Comment 6

[Chris Peterson [:cpeterson]]

Adding [qf] whiteboard tag so Quantum Flow triage will review this bug.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Hey florian, do you have an idea of how the ideas in this bug might impact start-up performance? Is this something we should be leaning on?

Comment 9

[Florian Quèze [:florian]]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #8)
> Hey florian, do you have an idea of how the ideas in this bug might impact
> start-up performance?

Loading the blocklist (_loadBlocklistFromString) after we are done with the I/O is 15% of the time spent before first paint on this startup profile of Firefox 58 on a slow Ubuntu machine: https://perfht.ml/2rTitpn

I think it's reasonable to assume the loading time is proportional to the entry count.

> Is this something we should be leaning on?

Yes!

Comment 10

[:Gijs (he/him)]

I contributed some code to kinto that will result in the blocklist xml served through our web service no longer including items where the <targetApplication> block matches the requesting app, but the maxVersion is less than the version of the requesting application. This code is not deployed yet, read on for more details. :-)

In other words, if Firefox 58 requests the blocklist and a blocklist item is for Fx 57, the server will not send this item.

This should allow us to mark all the items that are legacy addons as having a target application of Firefox, maxVersion 56.*, and from that point when all versions of Firefox next request a blocklist, it will omit all such items. This will easily shave 100kb off the on-disk size of the blocklist for Firefox 57+, without affecting 52 esr.

That was the plan / the good news.

The bad news is that effectively, the blocklist is shared with Thunderbird/SeaMonkey, and potentially other applications (it's a web service, anyone can make requests...). Most of the add-on items do not have a targetApplication listed right now. This means their blocks also apply on Thunderbird/SeaMonkey.

If we started adding a targetApplication of Firefox with a maxVersion of 56.*, those items will stop being sent to, and applying to, Thunderbird and/or SeaMonkey. In theory, that could put those users at risk.***

If we add a second targetApplication without a guid and maxversion="*" that would re-include those items, but, with current server code, would also send those items to Firefox again. Whether they apply to Firefox locally would depend on the order in which we specify the targetApplications, but clearly that's pretty fragile plus would obliterate any filesize improvements from the change. There'd be "no point" in doing this.

We have a few workable options:

1) deploy the current version of kinto and add targetApplication<=firefox56 to all the <emItem>. Ignore the fact that maybe some of these items want blocking on Thunderbird/SeaMonkey/wherever***
2) deploy the current version of kinto and add targetApplication<=firefox56 *and* TB/SM items for those items where the original blocked add-on would actually install on those apps. Andreas is looking at how many items in the current blocklist this would even cover. We suspect the number to be very close to 0. Ni for this.
3) deploy the current version of kinto and add targetApplication<=firefox56 *and* TB/SM items for *all* items. This will balloon the size of the blocklist on TB/SM because of all the additional <targetApplication> stuff.

[Note that for (1), (2), and (3), any non-TB/SM apps that rely on the blocklist will potentially be impacted - again, only if the respective add-on/plugin items actually install in those apps... We don't have a list of such potential apps to hand, though we could ask ops to use http logs to try to get one.]

4) add more code to amo2kinto that stops looking for other matching targetApplications once it finds one with a matching guid. This would allow the shortcut mentioned earlier, adding a wildcard targetApplication entry. We would still not send items to Firefox, but we'd send them to everybody else. This would still increase blocklist size significantly on those other clients, because of the additional <targetApplication> markup that'd be included (though I guess we can try to avoid this on the server-side, that'd add more complexity and thus room for error...)


Personally, while I'm happy to write the code for (4) if required, I would like to go for (2) (which I suspect devolves to (1), but we'll see!). See the caveats about the use of the blocklist for non-Firefox apps below. I'd like some feedback from Jorge, Dave and Mathieu as to how they feel on this issue (which might just be "do what you think is best", but I'm still curious! :-) ).


*** TBH, I'm not convinced how much value the add-on/plugin item blocklist adds to anything but Firefox. I don't think we've added Thunderbird-specific items in years. Without signing (and even with signing, given that uploading stuff to AMO will sign), IDs of annoying add-ons can just be changed, bypassing the blocklist. It's only slightly harder for plugins, where AFAICT it's as much a mechanism to stop (startup) crashes as anything else. I'm told TB/SM are migrating off AMO later this year, so I'm not convinced that being too worried about them is justified at this point.

Comment 11

[Andreas Wagner [:TheOne] [use NI]]

(In reply to :Gijs from comment #10)
> 2) deploy the current version of kinto and add targetApplication<=firefox56
> *and* TB/SM items for those items where the original blocked add-on would
> actually install on those apps. Andreas is looking at how many items in the
> current blocklist this would even cover. We suspect the number to be very
> close to 0. Ni for this.

The current blocklist has 456 add-on blocks in it. Out of those 456, 56 use a regular expression pattern to match the add-on GUID. Those were ignored for the following query.
Out of the 400 remaining, I checked which ones have at least one version that defined compatibility to Thunderbird, Seamonkey, Sunbird or Mobile (old Fennec), (or a combination of those).

11 add-ons match, here they are:

+----------------------------------------+
| guid                                   |
+----------------------------------------+
| [e-mail]         |
| [e-mail]                    |
| [e-mail] |
| [e-mail]             |
| [e-mail]                   |
| {46551EC9-40F0-4e47-8E18-8E5CF550CFB8} |
| {5C655500-E712-41e7-9349-CE462F844B19} |
| {7b1bf0b6-a1b9-42b0-b75d-252036438bdc} |
| {a0d7ccb3-214d-498b-b4aa-0e8fda9a7bf7} |
| {bee6eb20-01e0-ebd1-da83-080329fb9a3a} |
| {e1aaa9f8-4500-47f1-9a0a-b02bd60e4076} |
+----------------------------------------+

Comment 12

[Andreas Wagner [:TheOne] [use NI]]

Ok, we realized it's slightly more complicated than that since we need to take blocks for webextensions into account. And an add-on might have legacy versions, webextensions versions or both at the same time. I guess then it really depends on what add-on version the particular block applies to.

Comment 13

[Dave Townsend [:mossop]]

I don't have a lot to add here. If Thunderbird are particularly concerned they could consider forking the blocklist.

Two additional comments:

One of the reasons we originally decided to always include all block entries regardless of application version was to support the case of users downgrading. With all entries in the blocklist they would still have up to date blocks protecting them. Changing this might impact some users. I have no idea how prevalent this is though.

You're going to probably need to do work on the code that syncs the blocklist to the tree (https://searchfox.org/mozilla-central/source/browser/app/blocklist.xml) to make sure it is identifying the version it cares about to get the right blocklist.

Comment 14

[Florian Quèze [:florian]]

(In reply to Dave Townsend [:mossop] from comment #13)

> One of the reasons we originally decided to always include all block entries
> regardless of application version was to support the case of users
> downgrading. With all entries in the blocklist they would still have up to
> date blocks protecting them. Changing this might impact some users.

Would it make sense to move away from the blocklist xml file at the same time as we remove pre-webextension entries, so that users downgrading still have the full xml file in their profile?

Comment 15

[:Gijs (he/him)]

(In reply to Dave Townsend [:mossop] from comment #13)
> I don't have a lot to add here. If Thunderbird are particularly concerned
> they could consider forking the blocklist.
> 
> Two additional comments:
> 
> One of the reasons we originally decided to always include all block entries
> regardless of application version was to support the case of users
> downgrading. With all entries in the blocklist they would still have up to
> date blocks protecting them. Changing this might impact some users. I have
> no idea how prevalent this is though.

I think users who are on 56 at this point will do their darndest not to upgrade to 57+, so I'm not too worried about this. Even if it happens, they'd be running the 'wrong' list for <= 24 hours at most, which doesn't seem like a big deal.

> You're going to probably need to do work on the code that syncs the
> blocklist to the tree
> (https://searchfox.org/mozilla-central/source/browser/app/blocklist.xml) to
> make sure it is identifying the version it cares about to get the right
> blocklist.

I looked at this; it's using the version.txt information that's in the tree, so this is already correct, I think. :-)

(In reply to Florian Quèze [:florian] from comment #14)
> Would it make sense to move away from the blocklist xml file at the same
> time as we remove pre-webextension entries, so that users downgrading still
> have the full xml file in their profile?

For the reasons given above, I don't think we have to be overly careful with this and could drop the pre-webextension items for new Fx versions sooner rather than later.

Comment 16

[Marco Castelluccio [:marco]]

(In reply to :Gijs from comment #15)
> (In reply to Dave Townsend [:mossop] from comment #13)
> > I don't have a lot to add here. If Thunderbird are particularly concerned
> > they could consider forking the blocklist.
> > 
> > Two additional comments:
> > 
> > One of the reasons we originally decided to always include all block entries
> > regardless of application version was to support the case of users
> > downgrading. With all entries in the blocklist they would still have up to
> > date blocks protecting them. Changing this might impact some users. I have
> > no idea how prevalent this is though.
> 
> I think users who are on 56 at this point will do their darndest not to
> upgrade to 57+, so I'm not too worried about this. Even if it happens,
> they'd be running the 'wrong' list for <= 24 hours at most, which doesn't
> seem like a big deal.

IIRC we have a not-insignificant number of users who update very slowly (or that we block on
earlier versions temporarily because of some issue).
The blocklist sometimes prevents startup crashes, so we might lose some users if we
do this.

Comment 17

[:Gijs (he/him)]

(In reply to Marco Castelluccio [:marco] from comment #16)
> IIRC we have a not-insignificant number of users who update very slowly (or
> that we block on
> earlier versions temporarily because of some issue).

This change would only affect newer versions of Firefox... not users who are still on 56 or earlier. So I don't see what speed of updating has to do with it.

> The blocklist sometimes prevents startup crashes, so we might lose some
> users if we
> do this.

See above, but also, add-on or plugin blocklisting (of non-flash) on 58+ won't be there to prevent startup crashes, as webextensions should never cause crashes, and non-flash plugins are ignored.

Comment 18

[Marco Castelluccio [:marco]]

(In reply to :Gijs from comment #17)
> (In reply to Marco Castelluccio [:marco] from comment #16)
> > IIRC we have a not-insignificant number of users who update very slowly (or
> > that we block on
> > earlier versions temporarily because of some issue).
> 
> This change would only affect newer versions of Firefox... not users who are
> still on 56 or earlier. So I don't see what speed of updating has to do with
> it.
> 
> > The blocklist sometimes prevents startup crashes, so we might lose some
> > users if we
> > do this.
> 
> See above, but also, add-on or plugin blocklisting (of non-flash) on 58+
> won't be there to prevent startup crashes, as webextensions should never
> cause crashes, and non-flash plugins are ignored.

I thought users on 56 or earlier would lose some elements on the blocklist.
If that's not the case, forget what I said.

Comment 19

[Mathieu Leplatre [:leplatrem]]

Not much to add either, I'll follow your instructions. We are ready to deploy the new filtering code for the server on STAGE, waiting for your green light :)

Comment 20

[:Gijs (he/him)]

Talked with Jorge on IRC. Salient point:

[17:44:41]	Gijs	jorgev: so are you saying if we mark all the old stuff as target: Fx with maxVersion 56.*, so that neither TB/SM/Fennec nor modern Firefox will get it, that should be OK?
[17:45:18]	jorgev	yes
[17:45:36]	jorgev	if something comes up for those specific applications, we can always add blocks that target them

I'll file 2 deps to handle the actual updating of those items once the relevant kinto bits hit production.

Comment 21

[Kannan Vijayan [:djvj]]

Hi Gijs,

You mentioned you were going to file 2 dependencies for this (in comment 20).  Is there more work to be done here, or are we good to go?  It seems all the current blocking bugs have been closed.

Comment 22

[:Gijs (he/him)]

(In reply to Kannan Vijayan [:djvj] from comment #21)
> Hi Gijs,
> 
> You mentioned you were going to file 2 dependencies for this (in comment
> 20).  Is there more work to be done here, or are we good to go?  It seems
> all the current blocking bugs have been closed.

Hi, yes, this was waiting on bug 1437025 (new version of kinto being deployed to production). That happened about 20 minutes before your comment. I was out of the house at the time, so apologies for the delay. I'll file new dep bugs in a sec.

Comment 23

[:Gijs (he/him)]

The certs are already gone as of the production deploy an hour ago, so that's nice:

Before:
303449 blocklist.xml

After:
147483 blocklist.xml

Comment 24

[:Gijs (he/him)]

I filed the two deps. I expect that we can get the blocklist down from the ~145k it is now by:

- another ~28k by no longer sending unused plugin blocklist items (bug 1438006)
- remove somewhere between 0 and 100k by removing webextension blocklist items. I used to think that there weren't *any* webextensions in the list (so I thought we could remove ~all of it) ***. That turned out to not be true, but there isn't any way for me to tell how many of the items are webextensions, as this information isn't in the publicly served blocklist. Still, I would be surprised if more than half the items had to stay, which would save another ~50k. (bug 1438011)

The remaining ~14k is taken up by the graphics blocklist. I don't know how easy it would be to cut in that - because it's the smallest I've so far not really looked at it. If the gfx folks have ideas I'm happy to try to help.

I'm hoping that the AMO folks can take care of the 2 bugs I filed. I'm going to turn my attention more towards the client-side bugs at this point, as we really need to fix the local IO and processing side of things.


*** This was because up until the last deploy the last modified date on the blocklist was wrong (and sometime middle of 2017, so prior to there being lots of webextensions in release). The new deploy has also corrected the LM date served for the blocklist.

Comment 25

[:Gijs (he/him)]

(In reply to :Gijs from comment #24)
> - remove somewhere between 0 and 100k by removing webextension blocklist
> items.

Err, this should say *non-webextension* items, of course...

Comment 26

[Florian Quèze [:florian]]

Not sure if this is fxperf:p1 or a meta bug, Gijs?

Comment 27

[:Gijs (he/him)]

(In reply to Florian Quèze [:florian] from comment #26)
> Not sure if this is fxperf:p1 or a meta bug, Gijs?

I think it's basically a metabug at this point.

Comment 28

[Jean Gong :jgong]

change whiteboard to [qf:meta] since this bug is now a meta bug.

Comment 29

[Jorge Villalobos [:jorgev] (he/him)]

All dependencies have been resolved.