We could put parsed stylesheets in the cache to avoid reparsing them. When surfing a site you will probably reload the same CSS file(s) for pretty much every page you visit so being able to just grab a parsed stylesheet seems like a nice thing. There are of course some issues. First of all it's a perf vs. memory issue. Is it reall better to keep other things in the cache then two copies (the normal filestream and the parsed objects) then to just reparse the file every time? And is it worth the extra code? Second the cachecode needs to know the mem-size of the parsed stylesheet. We could probably estimate this by multiplying the filesize of the stylesheet with some constant (less then 1). Third we need code to deal with @import-ed stylesheets when calculating the expiredate. It shouldn't be too hard though, just take the date closest into the future. We could also simply skip stylesheets with @import rules. However my guess is that those are the stylesheets where we would save the most time. What do you guys think? Could this be worth it? Has anybody seen any profiles that show if we spend time in the css-loader/parser?

I think this has been filed before, but I can't find a duplicate. Related: bug 38486

one very important difference to bug 38486 though is that for stylesheets we're very likly to actually use the cached object since it's probably used on the next page or pages you go to.

> some constant (less then 1) Why less than 1? I would not be surprised if the parsed representation of a sheet is larger than the original plaintext file... In addition to @import rules we may need to watch out for background images, list-style images, etc, depending on what gets done in bug 113173 (we may end up using imgIRequest objects there instead of nsIURI objects).

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

Bobby, does stylo make this easier/harder? Or is it relevant at all to stylo?

bkelly says this has a huge impact on Facebook page load times.

(In reply to :Ehsan Akhgari from comment #5) > Bobby, does stylo make this easier/harder? Or is it relevant at all to > stylo? I think stylo is mostly orthogonal to this, though there's enough churn in the style system right now that we might want to hold off on doing more things there if we can avoid it. Stylo does give us the potential opportunity to parallelize CSS parsing, see bug 1346988. That's probably not going to be in the MVP though, where we're just shooting for parity on parsing performance.

(In reply to :Ehsan Akhgari from comment #6) > bkelly says this has a huge impact on Facebook page load times. I think I mispoke about FB. I believe I was looking at linkedin. This is less important for SPAs, but sites like washingtonpost.com, linkedin, etc that do real navigations between pages end up reloading stylesheets. Chrome avoids the need to even start a network request which makes these intra-site navigations feel a lot faster.

Note, I think a lot of the wins from this are not on raw cost of http cache lookup or css parse. Avoiding the need to perform an async operation and schedule another main thread runnable can have important latency benefits. Our main thread is very congested during loads, so anything we can do to avoid main thread runnables will probably pay off.

Ben, can you please provide an STR that can be profiled, or provide a profile yourself? Thanks!

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #7) > (In reply to :Ehsan Akhgari from comment #5) > > Bobby, does stylo make this easier/harder? Or is it relevant at all to > > stylo? > > I think stylo is mostly orthogonal to this, though there's enough churn in > the style system right now that we might want to hold off on doing more > things there if we can avoid it. Well, I'd like to first see a profile to understand what level of caching we're talking about... For example, naively thinking here, it seems to me that we could easily extend nsLayoutStylesheetCache with the ability to also cache stylesheets coming from web pages, if we know what the right cache key would be, when to evict entries, etc, and that sounds fairly simple to me and completely doable in parallel to Stylo given that it happens at a layer above Stylo. I'd be *super* interested to do something along those lines for QF if it wins us some perf and is tractable.

I'd also like to know what David thinks about this, since he may have thought about this before.

I can't immediately reproduce what I saw previously. Unfortunately I probably won't have time to investigate further in the near future. Feel free to ignore my previous statements on this.

The "when to evict entries" bit is somewhat complicated, because of the need to respect HTTP semantics, right? Imagelib does this by loading a stale thing and then replacing it with the new thing if HTTP says it should actually have changed, but that's a harder sell with CSS, I think. That said, it's possible other browsers don't respect HTTP semantics here; I don't know.

Let's at least investigate this for QF.

FWIW this impacts https://krakenbenchmark.mozilla.org/ which makes network requests for /kraken.css something like 140 times while running. (Based on bug 1352085 I suspect this bug impacts other Talos tests too.) (In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #14) > Imagelib does this by loading a stale thing and then replacing it > with the new thing if HTTP says it should actually have changed (Doing that for CSS is bug 510121.)

I think caching parsed style sheets in an nsExpirationTracker-based cache is a good idea, presuming that we can either respect HTTP semantics across pages, or that we're not the first browser not to do so. Once we do this, we should also file a followup bug on doing a bug 77999-equivalent in some cases for content style sheets.

Marking as [qf:p1] for an initial investigation on what other browsers do here, and whether we want to decide that we *don't* want to do this for QF.

We spoke a little bit about higher level caching like this and service workers at the spec meeting. We have previously done work in image cache to make sure service worker is consulted again instead of using the cache in some places. The current thinking, though, is its probably ok to allow this higher level caching as long as something like the "cache-control:no-store" header disables the cache. So, if this is moves forward I think we could avoid having to do anything special in regards to service workers as long as "no-store" is respected. At least to start, anyway.

Why the activity on this bug stopped a year ago? and never start again?

Two reasons: 1) It was investigated and decided that it wasn't a hard blocker for the goals we had at the time. 2) Time was focused on the new style system instead. This may still be worth doing, but needs measurements to see how worthwhile it is.

Another point is that, starting in Firefox 61 (released yesterday), we parse stylesheets off the main thread, in parallel with the other work during pageload. So unless the stylesheets are particularly big, or the user only has a single core, stylesheet parsing is much less likely to be the long pole in page rendering.

OMT parsing surely helps, but there is still a price to pay for opening a main thread nsIChannel to load the stylsheet bytes. In particular, I expect any MT jank that delays the nsIChannel runnables will delay the start of the OMT parsing. But yea, need measurements.