bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.

implement finer-grained sharing in XUL prototype cache

Assigned to



18 years ago
8 years ago


(Reporter: Chris Waterson, Assigned: Chris Waterson)


(Blocks: 1 bug, {memory-footprint, perf})

memory-footprint, perf

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [nsbeta3-] css and xbl done, js pending)


(2 attachments)



18 years ago
Currently, the XUL prototype shares XUL, JS and CSS at the level of a XUL 
document. In other words, we have one entry in the cache for each XUL document 
that is loaded, and the JS and CSS files are "attached" to that entry.

Because of this, warren & dougt discovered (in bug 46045) that many CSS and JS 
files are loaded repeatedly because they are "included" from several different 
overlay files.

It seems that the right thing to do here is to create additional caches (or, 
augment the existing "prototype cache") to allow JS and CSS to be cached 

This will improve startup and window-open time, as well as reduce overall 
footprint. According to dougt's trace in bug 46045, we load 96 JS and CSS 
files. Of those, 65 are unique. That means that, on startup, we are re-parsing 
31 files! In other words, a third of the files are read >1 time!


18 years ago
Keywords: nsbeta3, perf
Target Milestone: --- → M18


18 years ago
Keywords: footprint

Comment 1

18 years ago
*** Bug 46182 has been marked as a duplicate of this bug. ***

Comment 2

18 years ago
I thought scripts and stylesheets were cached independently.

Comment 3

18 years ago
Boioioioing. I just re-read this code, because it's been a while...

Out-of-line scripts are not independently cached (e.g., by the script's src= 
URL). I'm pretty sure that we could & should do this.

Style sheets look like they *should* be independently cached (contrary to my 
ramblings in the original report). I checked the content sink and XUL document 
code, and both look like they try to look up the sheet in the cache. The code 
looks like it would only load it from the filesystem if it can't find the sheet 
in the cache. For some reason, it's not working (see dougt's data in bug 46045): 
we are definitely re-parsing style sheets.

Comment 4

18 years ago
One thing I remembered... we never hooked into the style system to cache style 
sheets that are imported BY OTHER STYLE SHEETS.  Make sure you aren't just 
running into that.

e.g., we might keep loading global.css because other stylesheets import it.

Comment 5

18 years ago
I've got this one.

Observations on the navigator window: 
(1) It's the sidebar that loads duplicate files.  If you have the sidebar 
closed, no redundant stylesheets or scripts are loaded.  This is pretty sweet.
(2) The sidebar sucks in scripts and stylesheets that were already loaded by 
navigator.xul's load.  They are the exact two problems mentioned here, namely 
not caching scripts independently and the @import problem with stylesheets.

Once I take care of this, we should see a boost when loading windows of 
different types, e.g., messenger.css won't waste time reloading the global 
and communicator stylesheets once you've loaded navigator.
Assignee: waterson → hyatt

Comment 6

18 years ago
cc'ing pierre, because we talked with him about this today.

Comment 7

18 years ago
Created attachment 11985 [details] [diff] [review]
Patch to ensure all @import stylesheets are cached.

Comment 8

18 years ago
hyatt: looks good to me. but pierre or attinasi will probably want to review.


18 years ago
Whiteboard: fix for css needs review, js fix pending

Comment 9

18 years ago
Looking at the patch I notice a couple of things I'd like to bring up: 

1) Coupling the style system to XUL without '#ifdef INCLUDE_XUL' seems like it 
might be a mistake. I think CSSFrameConstructor is currently the only CSS class 
coupled to XUL and it is in #ifdef.  We should keep the CSS code free of 
specific knowledge of higher-level concepts like XUL, but at least #ifdef the 
use of XUL in the proposed patch for now. (If you think I am off-base in my 
desire to limit the coupling to XUL please state your case...)

2) (minor) The strings that represent the component URI's 
("component://netscape/rdf/xul-content-utils" and 
"component://netscape/rdf/xul-prototype-cache") should be constants defined once 
and used twice.

These points aside, the change looks good.

Comment 10

18 years ago
I seem to have caused a refcounting problem, so I'll have to look at this 

Comment 11

18 years ago
Created attachment 11988 [details] [diff] [review]
Better patch.  Added #ifdef and cloned cache-hit child import sheets.

Comment 12

18 years ago
Thanks for copying me on this. I'm looking into it.

Comment 13

18 years ago
This one may depend on / be related to bug #29370.

Comment 14

18 years ago
I'm ready to check in the CSS loader patch once I'm given the word from the CSS 
guys (pierre, attinasi, is the new patch good to go?)  It's working great on my 
machine and ensuring that our @imported chrome CSS gets into the XUL cache like 
the other stylesheets.

Comment 15

18 years ago
The patch looks fine to me - thanks for #ifdef'ing the XUL logic. 

I guess the cloning of the sheets is necessary to prevent a refcount probem that 
you mentioned? I don't quite get it though. It looks like you are getting a 
sheet from the cache, then cloning it, and then putting the cloned sheet into 
the cache (replacing the first sheet?). I guess I don't understand how the cache 
works. Anyway, the changes look fine if the cloning is really what you need to 
do (if you could explain it to me off-line I'd appreciate it).

Comment 16

18 years ago
*** Bug 46548 has been marked as a duplicate of this bug. ***


18 years ago
Whiteboard: fix for css needs review, js fix pending → [nsbeta3+] fix for css needs review, js fix pending

Comment 17

18 years ago
I partially agree with Marc. I think it is correct to clone the stylesheet out of 
the cache but:
- Why do we cache the cloned stylesheet too?
- Shouldn't we remove the stylesheet from the cache when it is deleted? Or how 
does the cache work in that regard?

Comment 18

18 years ago
Yeah, there's a dup issue in the caching that occurs because I haven't removed 
the style sheet caching code from the XUL side, i.e., the CSS loader is the only 
one that really needs to cache now.

I believe cloning gets involved because the parent sheets are different for two 
instances of the same stylesheet, so you have to clone the child.

or something like that... :)

Since it sounds good, I'm gonna check this in now becuase of the obvious 
performance win (and the nsbeta3+ marking), and then I'll go ahead and fine-tune 
it on the mozilla/rdf/content side of things to prevent the dup problem.

Comment 19

18 years ago
To answer your question, pierre, the chrome cache right now holds onto the 
stylesheets for the lifetime of the app (unless flushed out of the cache, which 
happens on a skin switch).

We will be implementing memory pressure APIs that will enable you to flush 
stylesheets selectively and to make the cache a little less of a memory-gobbler.


18 years ago
Whiteboard: [nsbeta3+] fix for css needs review, js fix pending → [nsbeta3+] fix for css checked in, js fix pending

Comment 20

18 years ago
I'm also  going to handle putting XBL into the chrome cache with this bug... so 
it now has three parts, CSS, JS, and XBL.  The first part is fixed.  The JS and 
XBL fixes are still to come.
Whiteboard: [nsbeta3+] fix for css checked in, js fix pending → [nsbeta3+] css done, js/xbl fixes pending

Comment 21

18 years ago
Ok, XBL now uses the chrome cache for chrome/resource URLs.
Whiteboard: [nsbeta3+] css done, js/xbl fixes pending → [nsbeta3+] css and xbl done, js pending

Comment 22

18 years ago
In an effort to spread love and sweet cheer, I'm gonna leave the JS caching to 
you, waterson.  Enjoy.
Assignee: hyatt → waterson

Comment 23

18 years ago
Fixing the XUL cache to cache JS will save us from loading one extra JS file out
of 22 when bringing up the first navigator window.

(FWIW, The file is chrome://global/content/builtinURLs.js, which probably is
only being loaded twice because it's a bug in the inclusion scheme. It's
included from both navigator.xul and utilityOverlay.xul. ben, is this necessary?
I'll file a separate bug if not.)

I'm going to future and [nsbeta3-] the "remnants" of this bug. We may want to
cache JS individually some day, but for now it's not going to buy us much.
Whiteboard: [nsbeta3+] css and xbl done, js pending → [nsbeta3-] css and xbl done, js pending
Target Milestone: M18 → Future


17 years ago
Blocks: 104400


16 years ago
Keywords: nsbeta3


10 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.