385260 - getComputedStyle performance 20x worse than Safari for Windows

Status: RESOLVED WORKSFORME

(Core :: DOM: CSS Object Model, defect, P5)

Description

[Brandon DuRette]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070620 Minefield/3.0a6pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070620 Minefield/3.0a6pre

The Ext Javascript library makes a number of calls to getComputedStyle when performing layouts, but then only examines a limited number of properties on the result (i.e., border widths, padding, overflow).  With a fairly simple layout, but one that contains a large table (~1000+ rows by 5 columns) in one panel (div), the layout takes ~7 seconds in Firefox, but only ~350 millisecs in Safari.  Using Firebug's profiler, I was able to identify getComputedStyle as the major consumer of time in Firefox and while invoked in Safari, it takes almost no time at all.  

From reading various mailing list threads on the issue (both Mozilla and Ext threads), I came to the conclusion that Firefox is sometimes forced to do what amounts to a complete layout to compute all the styles.  This is fine, but most of that information is thrown away almost immediately and it seems to me that the particular attributes that are requested do not require a full layout -- just a full CSS cascade.  Therefore, if getComputedStyle was lazy about getting the values, then a full layout would not be required in some cases.

Reproducible: Always

Steps to Reproduce:
1. Untar the attached test case
2. Open test.html in Firefox
3. Observe the layout time which is alerted upon completion
4. Compare to Safari (Windows)

Workaround: If the table's display style is set to "none" for the duration of the layout, Firefox's performance difference becomes negligible, though Safari still wins.  There is a line in layout.js which is commented out which does just that.  Just uncomment that line to observe the difference.
Actual Results:  
Example results: 

Firefox layout time:  7641ms
Safari layout time:  297ms

Expected Results:  
Firefox should perform in the same order of magnitude as Safari for this operation.

Comment 1

[Brandon DuRette]

Attachment: Test case, including the required Ext.js library

Comment 2

[Boris Zbarsky [:bzbarsky]]

If you're asking for padding, that requires a full layout....

Comment 3

[Boris Zbarsky [:bzbarsky]]

So I just did a profile of that testcase.

Here are the things that are being queried during the measured time that require a layout or style recomputation.  All percentages are time spent in that operation as a percentage of the total time measured on the page:

  .clientWidth (3% reflow, 13% restyle)
  Getting computed right border width (6% reflow, 8% restyle)
  Getting computed top border width (12% reflow , 14% restyle)
  Getting computed left border width (15% reflow, 16% restyle)

That adds up to 87% of total time.  Border widths involve reflow because the actual layout widths (which are what Gecko returns) might not match the style data in some subpixel layout cases.

Of course if getComputedStyle actually returned CSS2.1 computed style you would be right about only having to do the cascade.  But it actually returns CSS2.1 used style (which is closer to CSS 2 computed style)...

All that said, over 50% of the time is in fact spent reresolving style data.  Are you mutating styles on things near the root of the tree in between the getComputedStyle calls or something?

Confirming, but my gut feeling is that this is tickling a rare case that we just happen to not optimize for very well.

Comment 4

[Steve Roussey (:sroussey)]

Boris, if you kick this up a notch I promise to add some things you requested to Firebug!

It makes one's heart sink when IE6 is faster than Firefox 3.6!

Take this page for example:
http://www.extjs.com/deploy/dev/examples/portal/portal.html

If I resize the Firefox window, it gets real jerky and slow. IE6 is far far faster and smoother -- and that is IE6 running in a virtual machine! Seriously! I now completely understand when M$ talks about JS and JIT not mattering much if everything else not up to the same speed characteristics.

What can be done?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Steve, IE doesn't implement getComputedStyle, so the site is doing something totally different in IE.  I have no idea _what_, nor how one would expect it to perform.

Safari and Opera over here lag just as much as Gecko does, so my current impression is that the non-IE code on that page is just written in a particularly stupid way.

Oh, and I should note that an engine that has better CSS support is almost certain to be slower than one that does not at this sort of testcase, because the more advanced CSS features invalidate optimizations that are possible if you don't support them; I'm not sure IE6 is a useful metric in that regard.

> What can be done?

If you really feel that making this demo page faster is important, the most useful thing is an analysis of what exactly it's doing and why in its js.  There's a chance that other restyle optimizations we're working on anyway would help some, but fundamentally it looks like this page keeps changing styles and then reading layout data, which is just going to be slow as long as the "reading layout data" step doesn't lie about the styling and layout.

Comment 6

[Steve Roussey (:sroussey)]

Well, I have my opinions on what the code does and what should change, but I don't have ownership over it, and already maintain my own fork. But I am a big user of the framework, so I have a vested interest in seeing it improve, and big user of Firefox and thus want it to work on fav as fast as it can... :)

I definitely get that a CSS3 engine has more to do than a CSS1 one. I'm just a bit surprised that the JS framework was able to get the IE version working so well (better in this case) despite all of IE's shortcomings and failures.

I know their code interleves measurements with setting dom stuff, causing redundant layout, etc., before a browser repaint. There is definitely work that can be done.

Perhaps it is because the framework tends toward the border box model internally that it works faster with IE. I have suggested that they use the box-sizing css property where they can so they can hopefully take less measurements of the padding and border widths. Those account for most of the getStyle calls (85%-90%).

Also, is the result of getComputedStyle a 'live' object? Does that matter? And if I were going to get 4-5 values out of it, does getting the object new each time make a significant difference?

	getComputedStyle(dom,"")['borderLeftWidth']
	getComputedStyle(dom,"")['borderRightWidth']

vs

	cs = getComputedStyle(dom,"")
	cs['borderLeftWidth']
	cs['borderRightWidth']

?

For reference, the getComputedStyle is used in getStyle found further below:

	view = document.defaultView;
	
	function chkCache(prop) {
	    return propCache[prop] || (propCache[prop] = prop == 'float' ? propFloat : prop.replace(camelRe, camelFn));
	}
	...
	
	getStyle : function(){
	    return view && view.getComputedStyle ?
	        function(prop){
	            var el = this.dom,
	                v,
	                cs,
	                out,
	                display,
	                wk = Ext.isWebKit,
	                display;
	                
	            if(el == document){
	                return null;
	            }
	            prop = chkCache(prop);
	            // Fix bug caused by this: https://bugs.webkit.org/show_bug.cgi?id=13343
	            if(wk && /marginRight/.test(prop)){
	                display = this.getStyle('display');
	                el.style.display = 'inline-block';
	            }
	            out = (v = el.style[prop]) ? v :
	                   (cs = view.getComputedStyle(el, "")) ? cs[prop] : null;
	
	            // Webkit returns rgb values for transparent.
	            if(wk){
	                if(out == 'rgba(0, 0, 0, 0)'){
	                    out = 'transparent';
	                }else if(display){
	                    el.style.display = display;
	                }
	            }
	            return out;
	        } :
	        function(prop){
	            var el = this.dom,
	                m,
	                cs;
	
	            if(el == document) return null;
	            if (prop == 'opacity') {
	                if (el.style.filter.match) {
	                    if(m = el.style.filter.match(opacityRe)){
	                        var fv = parseFloat(m[1]);
	                        if(!isNaN(fv)){
	                            return fv ? fv / 100 : 0;
	                        }
	                    }
	                }
	                return 1;
	            }
	            prop = chkCache(prop);
	            return el.style[prop] || ((cs = el.currentStyle) ? cs[prop] : null);
	        };
	}(),

Comment 7

[Boris Zbarsky [:bzbarsky]]

> Get the IE version working so well (better in this case)

That's because you're comparing apples to oranges.  The framework is doing totally different things in IE and non-IE browsers, per your paste.

> Also, is the result of getComputedStyle a 'live' object?

Yes.

> Does that matter?

Probably not.

> And if I were going to get 4-5 values out of it, does getting the object new
> each time make a significant difference?

Not in Gecko at the moment.  I can't speak to other UAs.

> For reference, the getComputedStyle is used in getStyle found further below:

So this runs completely different code for IE and Gecko/Safari/Opera, right?  For IE it queries the CSS 2.1 computed style (or equivalent); I'm not sure whether IE makes any guarantees about currentStyle correctness if recent mutations have happened.  In any case, this requires no relayout and due to IE's lack of support for the "inherit" keyword may not require much restyling in most cases.

For other browsers it queries the CSS 2 computed style, which is closer to the CSS2.1 used style than to the CSS2.1 computed style.  This does require relayout, and full restyling of the document.  It also has that nasty display thing going on for webkit that's going to slow it down even more.

Fundamentally, the code above is doing things like this:

  if (isIE) { get some data; }
  else { get some other data that requires a lot more computation and is NOT
         equivalent to the data gotten for IE; }

and your complaint seems to be that the isIE branch is faster... well, yes, of course it is!

Comment 8

[Steve Roussey (:sroussey)]

Setting isIE=true on Firefox still gives worse performance, though that is not too much the issue. But anyway, I guess the thing I can gleam out of this conversation (thank you for engaging in it), is that Gecko "requires relayout, and full restyling of the document" in our cases. How does one avoid that?

Now, for IE, it is getting the CSS2.1 computed style by "el.style[prop] || ((cs = el.currentStyle) ? cs[prop]"...

What about getComputedStyle states that it is CSS2 computed style / CSS2.1 used style? First off, when I look at the spec I think explains it, there doesn't seem to be a difference between CSS2 and CSS2.1 -- they look the same, so I'm missing something:

http://www.w3.org/TR/CSS2/cascade.html#computed-value
http://www.w3.org/TR/CSS21/cascade.html#computed-value

Second, I can see the issues if getComputedStyle is really getUsedStyle but misnamed. I'm sure the reality is far more cloudy than it being either this or that. But I do think that the JS frameworks are all moving to the point of having their own layout handlers, and those will all want to know the box-sizing and content-sizing height and width. So an optimized code path for these will show some speed increases in a lot of places.

BTW, with getComputedStyle being a live object, does it actually do the force the relayout when called, or when a specific property is accessed that requires it?

Comment 9

[Steve Roussey (:sroussey)]

Ug, such poor Engish... where is the preview button??!

Comment 10

[Steve Roussey (:sroussey)]

Just a note on docs:

https://developer.mozilla.org/en/DOM/window.getComputedStyle

Docs for getComputedStyle should be more explicit about returning used style. Instead, I only noticed it here:

https://developer.mozilla.org/en/CSS/computed_value

I suppose there is no "real" getComputedStyle?

Comment 11

[Boris Zbarsky [:bzbarsky]]

> How does one avoid that?

Don't change styles in between querying style and/or geometry information.

> What about getComputedStyle states that it is CSS2 computed style

The fact that the API was created when CSS2 was the only CSS on the block, that browsers then implemented it in terms of CSS2 computed style, and that web sites now depend on this behavior, such that changing it would break them.  For example, actually computing widths to "auto" in getComputedStyle would be a deal-breaker.

> http://www.w3.org/TR/CSS2/cascade.html#computed-value

Wrong URL.  You want http://www.w3.org/TR/2008/REC-CSS2-20080411/cascade.html#computed-value

> I can see the issues if getComputedStyle is really getUsedStyle but
> misnamed

It's not quite.  It's getUsedStyle for some properties, but not for others.  The whole thing is a bit of a mess.

> So an optimized code path for these will show some speed increases in a lot
> of places.

We haven't found a sane way yet to optimize geometry requests (of any kind) short of performing full layout on the document if any part is dirty.  Anything else risks all sorts of weird bugs, since various parts of the document can affect the geometry of other parts.

> BTW, with getComputedStyle being a live object, does it actually do the force
> the relayout when called, or when a specific property is accessed that
> requires it?

In Gecko, the latter (assuming the document is dirty; if it's clean there's nothing to do, of course).

> Docs for getComputedStyle should be more explicit about returning used style.

It's a wiki.  Fix as desired!  Note that the upcoming CSSOM spec will be very explicit about the behavior here.

> I suppose there is no "real" getComputedStyle?

Not yet.

Comment 12

[Steve Roussey (:sroussey)]

> It's not quite.  It's getUsedStyle for some properties, but not for others. 
> The whole thing is a bit of a mess.

I can see that now... ;) And the new spec seems to drop the term "used style" as well...

> > Docs for getComputedStyle should be more explicit about returning used style.
> It's a wiki.  Fix as desired! 

Never noticed that...

> > I suppose there is no "real" getComputedStyle?
> Not yet.

Hmm, looking at MS for currentStyle, it looks like they provide the computed style based at the point of the last repaint (not guaranteeing that it is currently correct). For a lot of things, this is actually preferable. Particularly when you know these values are not going to change, possibly ever, then you just want to get some values and move on (without relayouts).

So, God help me for asking this, can a similar to currentStyle thing be added to Gecko?

Comment 13

[Boris Zbarsky [:bzbarsky]]

> not guaranteeing that it is currently correct

That's a lot cheaper, obviously... ;)

> can a similar to currentStyle thing be added to Gecko

Sort of.  Gecko computes style lazily, so it might be possible to do it in many cases but you might still end up needing restyles in other cases... and so sometimes it'll return the old style and sometimes the new style.  File a bug if you think this is worth working on, in any case?

Comment 14

[Steve Roussey (:sroussey)]

OK, added here: https://bugzilla.mozilla.org/show_bug.cgi?id=544343

Comment 15

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.

Comment 16

[Gregory Pappas [:gregp]]

Attachment: brandons_testcase_hosted.html

Comment 17

[Gregory Pappas [:gregp]]

Attachment: chromium_firefox_gnome_web.png

Seems fine to me now.

Comment 18

[Gregory Pappas [:gregp]]

I cannot reproduce 7641ms layout in any build of Firefox, the farthest that mozregression took me is a beta of Firefox 3, in which I got 2903ms, maybe if I used period appropriate hardware It'd be slower.

Next, I was led to this this range which is Firefox 42, in which I got 991ms.

Then, we reach Firefox 55, where we get a cool 674ms.

mozregression finally led me to this range which is Firefox 57, where we get a perfect 333ms. Since then, Firefox has gotten a bit faster, with a cool 169ms being shaven off, for a final 164ms in Firefox 102. Sadly, WebKit has seemingly regressed in this case, I get somewhere between 800 and 1050ms in GNOME Web (WebKitGTK 2.36.1)

Emilio, can this be closed and what do you think fixed this?

Comment 19

[Emilio Cobos Álvarez [:emilio]]

Yeah, there have been a variety of optimizations that have landed over the years (bug 1404140 and bug 1363805 come to mind) which should avoid layout in this test-case, let's resolve as WFM.