1236605 - Talos regression ts_paint/paint on win x64 PGO/Non-PGO opt e10s/non-e10s - 3-3.6%

Status: RESOLVED FIXED

(Firefox :: Pocket, defect)

Description

[:Gijs (he/him)]

Talos sent me this email:



Regression: Fx-Team-Non-PGO - Ts, Paint - WINNT 6.2 x64 (e10s) - 3.03% increase
-------------------------------------------------------------------------------
    Previous: avg 418.521 stddev 5.362 of 12 runs up to revision 5a1d2aedd97fdb742ee1352b2567c293ba096fb8
    New     : avg 431.184 stddev 4.275 of 12 runs since revision ff863d8cdf7ad551bc4843e3614e09236400a6da
    Change  : +12.663 (3.03% / z=2.362)
    Graph   : http://mzl.la/1Svm3KO

Changeset range: http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=5a1d2aedd97fdb742ee1352b2567c293ba096fb8&tochange=ff863d8cdf7ad551bc4843e3614e09236400a6da

Changesets:
  * http://hg.mozilla.org/integration/fx-team/rev/fe8054f2ac3e
    : Margaret Leibovic <[e-mail]> - Bug 1235876 - Check scheme before handling lightweight theme events. r=Gijs
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1235876

  * http://hg.mozilla.org/integration/fx-team/rev/9126f599f632
    : Mark Finkle <[e-mail]> - Bug 1235642 - Don't query-then-update to bump history visit counts r=rnewman
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1235642

  * http://hg.mozilla.org/integration/fx-team/rev/ff863d8cdf7a
    : Gijs Kruitbosch <[e-mail]> - Bug 1235845 - add the pocket stylesheet to browser windows instead of using the stylesheet service, r=mixedpuppy
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1235845

Bugs:
  * http://bugzilla.mozilla.org/show_bug.cgi?id=1235642 - Don't query-then-update to bump history visit counts
  * http://bugzilla.mozilla.org/show_bug.cgi?id=1235876
  * http://bugzilla.mozilla.org/show_bug.cgi?id=1235845 - Pocket icon of toolbar button is broken on windows and osx HiDPI



Fairly sure that's bug 1235845 as the other bugs are android-only.

I can only guess that adding these stylesheets causes churn on startup. That is unfortunate. I'll see if there's an alternative that's less sad-making. Because the manifest is bootstrapped we can't use |style| registrations, which is unfortunate. That should actually be fixable, but I don't really know if that's a rabbithole we should want to go down here and now.

OTOH if this is non-PGO only and doesn't affect tpaint then maybe we should just swallow the regression... that said, it is also weird that so far this is e10s-only. Would be interesting to see what's happening on non-e10s but I don't have time to look at that right now.

Comment 1

[Shane Caraveo (:mixedpuppy)]

BTW, loop also uses the stylesheet service.

Comment 2

[:Gijs (he/him)]

(In reply to Shane Caraveo (:mixedpuppy) from comment #1)
> BTW, loop also uses the stylesheet service.

tbh, the whole thing is kind of weird in that previously the stylesheets should also be affecting the window, so it's odd that this way of specifying them would cause a regression, but there we are.

Comment 3

[:Gijs (he/him)]

This is also showing up elsewhere, but still only on Windows.

Comment 4

[:Gijs (he/him)]

Joel, did you figure out whether this was genuinely caused by bug 1235845?

Comment 5

[William Lachance (:wlach)]

It looks like Joel did a bunch of retriggers, and the answer seems to be "yes". e.g.:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=fx-team&originalRevision=5a1d2aedd97f&newProject=fx-team&newRevision=ff863d8cdf7a&framework=1&filter=tpaint%20windows&showOnlyConfident=1

Looking at the graph seems to confirm this isn't an intermittent issue or fluke:

https://treeherder.mozilla.org/perf.html#/graphs?series=[fx-team,38229d537cd5265f429b0916d523305fa6ce1993,1]&highlightedRevisions=5a1d2aedd97f&highlightedRevisions=ff863d8cdf7a

Comment 6

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

in addition we have tpaint and session restore regressions (ignore the osx 10.10):
https://treeherder.allizom.org/perf.html#/compare?originalProject=fx-team&originalRevision=5a1d2aedd97f&newProject=fx-team&newRevision=ff863d8cdf7a&framework=1&filter=tpaint&showOnlyConfident=1

session restore no auto restore:
https://treeherder.allizom.org/perf.html#/compare?originalProject=fx-team&originalRevision=5a1d2aedd97f&newProject=fx-team&newRevision=ff863d8cdf7a&framework=1&filter=restore&showOnlyConfident=1

Comment 7

[:Gijs (he/him)]

I have an idea what's going wrong here. I suspect it's because we don't add our things until browser-delayed-startup finishes. I anticipate that doing this earlier (domwindowopened, maybe then wait for DOMContentLoaded) will be better. I'll investigate more tomorrow.

Comment 8

[:Gijs (he/him)]

https://treeherder.mozilla.org/perf.html#/compare?originalProject=fx-team&originalRevision=5a1d2aedd97f&newProject=try&newRevision=bd95430e2e74

Comment 9

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1236605 - Fix pocket style addition performance, r?mixedpuppy

Review commit: https://reviewboard.mozilla.org/r/29911/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29911/

Comment 10

[:Gijs (he/him)]

Need to wait for this to finish:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd95430e2e74

From local testing, I *think* this is OK. But the numbers of ts_paint are very unstable locally. sessionstore tests are better, but still not great.

Comment 11

[:Gijs (he/him)]

Comparing to pre-regression looks good:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=fx-team&originalRevision=5a1d2aedd97f&newProject=try&newRevision=bd95430e2e74&framework=1&showOnlyImportant=0&showOnlyConfident=1

as does comparing to the regression (which makes the re-gains on some fronts, like e10s ts_paint, more explicit):

https://treeherder.mozilla.org/perf.html#/compare?originalProject=fx-team&originalRevision=ff863d8cdf7a&newProject=try&newRevision=bd95430e2e74&framework=1&showOnlyImportant=0&showOnlyConfident=1

So I'll land this and call it fixed... as soon as the tree reopens. :-\

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/46d9ba5a8e7c

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/46d9ba5a8e7c

Comment 14

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

verified this is fixed, thanks for doing it!

Comment 15

[Mayur Patil]

[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected