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 independently. 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!

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

I thought scripts and stylesheets were cached independently.

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.

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.

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.

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

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

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.

I seem to have caused a refcounting problem, so I'll have to look at this further.

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

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

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.

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).

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

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?

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.

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.

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.

Ok, XBL now uses the chrome cache for chrome/resource URLs.

In an effort to spread love and sweet cheer, I'm gonna leave the JS caching to you, waterson. Enjoy.

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.