Modify AMO review checklist to disallow add-ons which disable the XUL cache


Status Graveyard
6 years ago
2 years ago


(Reporter: Ehsan, Assigned: eviljeff)


Q1 2012


(Whiteboard: [MemShrink:P3])

So I discovered today that I have XUL cache disabled.  I'm seeing extremely high memory usage and extremely long GC/CC pauses.  I know that I have not set the nglayout.debug.disable_xul_cache pref myself, so this got me curious.  Look and behold:

Any of these add-ons which is not focused on debugging XUL is broken and should be disabled.
This is a major degradation of product quality and needs to be addressed.
Severity: normal → major
For those of us not in the know, what does the XUL cache do?
It's an in-memory cache of chrome stylesheets, XUL prototype documents, and XBL prototype bindings.

The main effect is that you don't have to keep loading/parsing these things; you just use the existing in-memory representation every time they're referenced.  This saves both CPU time and memory.
Assignee: nobody → jorge

Comment 4

6 years ago
I've not checked every match but a lot of these addons are old (max 3.6) or the preference is commented out so it might not be as widespread as it appears.  The validator now flags every preference set outside the "extensions." branch so we should catch any updates also.
OS: Mac OS X → All
Hardware: x86 → All
In today's MemShrink meeting we talked with the AMO guys about ways to flag add-ons as causing poor performance.  Combine that with comment 4, this seems like a low priority thing.  If we can identify any particular popular add-ons that disable the XUL cache, that might make it higher priority.
Whiteboard: [MemShrink] → [MemShrink:P3]
We won't blocklist add-ons based on this, but we will adjust our policies so that we check for this preference more attentively. I filed bug 725465 to add a validator check for this preference, and we'll investigate all add-ons that modify this preference.
Component: Administration → Policy
Depends on: 725465
QA Contact: administration → policy
Summary: Block add-ons which disable the XUL cache → Disallow add-ons which disable the XUL cache
Target Milestone: --- → Q1 2012
Version: ACR-0.9 → unspecified
I think we can WONTFIX this -- bug 725465 sounds good, but this bug doesn't really depend on it.
Last Resolved: 6 years ago
No longer depends on: 725465
Resolution: --- → WONTFIX
No, we still want this.
Resolution: WONTFIX → ---
Sorry, I misunderstood.  To clarify:

- This bug is about changing the AMO policy to disallow add-ons that disable the XUL cache.

- Bug 725465 is about adding an automated check for this in the add-on validation tool.
Depends on: 725465
Summary: Disallow add-ons which disable the XUL cache → Modify AMO review checklist to disallow add-ons which disable the XUL cache
Andrew: please look into all MXR results for this preference and send me a list of the add-ons that use it that are compatible with 4 and above.
Assignee: jorge → awilliamson

Comment 11

6 years ago
okay (I thought I'd post here so everyone is aware of the scale). Fully reviewed indicated with -F, Preliminary with -P.

The following add-ons could be classed as 'specialist', in that they are either designed to set up a development environment or expose 'hidden' Firefox prefs with a UI.  Unless we want to get really strict I'd say these are valid use-cases: -F -F -F -F -F -P -F -F

The following don't have any reason to set the preference: -P -P -P -F

Only the last once, clickclean, is fully reviewed so requires action.

(Technically, does it too but only in installed apps.)

Comment 12

6 years ago
I've contacted the developers of the four addons that set the preference without any need to requesting they remove XUL cache disable preference.  I've additionally downgraded the fully reviewed addon, clickclean, to preliminary reviewed.
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
From comment 9:
> - This bug is about changing the AMO policy to disallow add-ons that disable the XUL cache.

Has the AMO policy been updated as well?
The validation in bug 725465 should be enough for reviewers to do the right thing.
Product: → Graveyard
You need to log in before you can comment on or make changes to this bug.