946710 - dromaeo css regression on windows 7 and 8, pgo builds only

Status: RESOLVED WORKSFORME

(Core :: CSS Parsing and Computation, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

found on dev.tree-management:
https://groups.google.com/forum/#!topic/mozilla.dev.tree-management/8RsJl68G0AM

here are links to the graphs:
http://graphs.mozilla.org/graph.html#tests=[[72,63,25],[72,63,31]]&sel=1385646881246,1386251681246&displayrange=7&datatype=running

and datazilla links:
https://datazilla.mozilla.org/?start=1385242422&stop=1386251143&product=Firefox&repository=Mozilla-Inbound&os=win&os_version=6.1.7601&test=dromaeo_css&graph_search=196c1863a6ec&error_bars=false&project=talos

the datazilla links show that we show significant problems on dojo.html and prototype.html:
http://hg.mozilla.org/build/talos/file/2bcf422011d1/talos/page_load_test/dromaeo/tests/cssquery-prototype.html
http://hg.mozilla.org/build/talos/file/2bcf422011d1/talos/page_load_test/dromaeo/tests/cssquery-dojo.html

Looking at tbpl, we did a good deal of pgo builds, so the regression range for this is really small:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=48a03977660e&tochange=196c1863a6ec

this appears to be bug 944493, :heycam, can you confirm this and look into this bug?

Comment 1

[Boris Zbarsky [:bzbarsky]]

Fwiw, I saw that too, and it makes no sense: this change should cause no behavior difference....

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

there were some unified build changes as well between this and the last pgo build.

Comment 3

[Cameron McCormack (:heycam)]

I don't think bug 944493 would have caused the regression; it's a very simple string change.

Comment 4

[Mats Palmgren (inactive)]

> the datazilla links show that we show significant problems on dojo.html and prototype.html

It seems we got that perf back on 2013-12-10, for precisely those two tests:
https://datazilla.mozilla.org/?start=1385242422&stop=1387395084&product=Firefox&repository=Mozilla-Inbound&os=win&os_version=6.1.7601&test=dromaeo_css&graph_search=196c1863a6ec&error_bars=false&project=talos

is there an explanation for that event?

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

indeed this is fixed, here is the range of changesets for the fix (this is pgo, so there is a range):
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=673f57bf0ff7&tochange=250fe25bff9f

if needed we could probably force a pgo build on some of the intermediate pushes:

Comment 6

[Mats Palmgren (inactive)]

I don't see anything in that range that explains it.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

I kicked off some pgo builds and retriggers of win7 dromaeo runs, it is now narrowed down to:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=673f57bf0ff7&tochange=ec30593d13ba

any thoughts on the smaller range?

for reference the tbpl view:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&fromchange=673f57bf0ff7&tochange=ec30593d13ba&jobname=Windows%207%2032-bit%20mozilla-inbound%20pgo%20talos%20dromaeojs

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

I triggered pgo builds for the set inbetween and found this changeset:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f15bd9ef9b7e

this is from bug 946877.  bsmedberg, can you figure out why your change caused a noticeable regression on windows 7 pgo only dromaeo css tests?  I have done a series of retriggers before/during/after your changeset and it is pretty obvious, see links in comment 7.

Comment 9

[Benjamin Smedberg]

This cset caused all sorts of interesting PGO effects. Note that when it landed on beta, we saw a 5-10% improvement on various tests:
* a11y Row Major MozAfterPaint
* Paint
* Tp5 Optimized Responsiveness
* Tp5 Optimized

I understand from mbrubeck that similar improvements were seen when this landed on inbound, although because of the infrequency of PGO builds on inbound he didn't immediately associate them with this unlikely change.

And to be clear, I didn't expect this to have any perf implications at all. But given what we currently know, I'm ok with trading off regressions in Dromaeo CSS for Tp5 for now.

I already filed bug 951863 for the new year to investigate why we improved performance, and so I'm going to mark this a duplicate of that one and have dmajor investigate why it hurt performance also.

Comment 10

[Matt Brubeck (:mbrubeck)]

Comment 5 and comment 7 list *improvement* ranges, which means that bug 946877 *improved* the Dromaeo CSS performance.  That means that comment 8 and 9 are incorrect.  The regression happened before bug 946877 landed and is not a duplicate of bug 951863 (although bug 951863 did cause an *improvement* that canceled out the previous regression).

See comment 0 for the actual regression range.  According to that, the regression was caused by bug 943995 and/or bug 943993.

Dromaeo CSS has been fairly sensitive to minor changes in the past, so it might just be that seemingly random changes in our PGO builds are having outsized effects on its performance.

Comment 11

[(Away)]

In bug 951863 comment 8, I found some intermittent code-gen problems in recent PGO builds. Even if this bug isn't strictly a duplicate, I think they are definitely related.

Comment 12

[Mats Palmgren (inactive)]

Seems like we got our perf back so can we just resolve this as WFM?

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

I vote for closing this- we are back to normal.

Comment 14

[Matt Brubeck (:mbrubeck)]

Sounds good.  Tentatively chalking this up to PGO gremlins ala bug 951863.