922207 - Find out why UX branch regressed when bug 918613 landed

Status: RESOLVED WORKSFORME

(Firefox :: Tabbed Browser, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Looking at Datazilla this morning, I see a slight up-tick on several Windows XP TART subtests - specifically:

icon-open-DPI1.all.TART went from an average of 2.39 to 2.59
icon-open-DPI1.half.TART went from an average of 2.04 to 2.19
iconFade-open-DPI2.all.TART went from an average of 2.27 to 2.44
iconFade-open-DPI2.half.TART went from an average of 2.43 to 2.59
newtab-open-preload-no.all.TART went from an average of 2.97 to 3.09
newtab-open-preload-no.half.TART went from an average of 2.13 to 2.26
newtab-open-preload-yes.all.TART went from an average of 2.50 to 2.63
newtab-open-preload-yes.half.TART went from an average of 2.13 to 2.26
simple-open-DPI1.all.TART went from an average of 2.32 to 2.49
simple-open-DPI1.half.TART went from an average of 2.03 to 2.18

The spike is more obvious when looking at Datazilla graphs:

https://datazilla.mozilla.org/?start=1377082034&stop=1380560215&product=Firefox&repository=Firefox-Non-PGO&os=win&os_version=5.1.2600&test=tart&compare_product=Firefox&compare_repository=UX-Non-PGO&project=talos

Here are the changesets in that range:

https://hg.mozilla.org/projects/ux/pushloghtml?startID=467&endID=468

I'm immediately suspicious of:

3287d7bbdc14	Bas Schouten — Bug 918613: Convert cairo path code to use cairo_path_t. r=jrmuizel

Which just landed to fix bug 919656 (among other things).  I've pushed a patch to try that backs this out, and we'll see if these numbers go back down.

UX Baseline: https://tbpl.mozilla.org/?tree=Try&rev=647db2708384
UX + Backout: https://tbpl.mozilla.org/?tree=Try&rev=9a9e22855e71

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Backing out bug 918613 seems to result in a win:

http://compare-talos.mattn.ca/breakdown.html?oldTestIds=30005317,30008601,30008609,30008639,30008647,30008675,30008687,30008715&newTestIds=30005307,30008593,30008617,30008625,30008695,30008707,30008807&testName=tart&osName=Windows%20XP&server=graphs.mozilla.org

So I conclude that something about bug 918613 slowed UX down on XP.

Any idea what that might have been, Bas?

Comment 2

[Bas Schouten (:bas.schouten)]

(In reply to Mike Conley (:mconley) from comment #1)
> Backing out bug 918613 seems to result in a win:
> 
> http://compare-talos.mattn.ca/breakdown.html?oldTestIds=30005317,30008601,
> 30008609,30008639,30008647,30008675,30008687,30008715&newTestIds=30005307,
> 30008593,30008617,30008625,30008695,30008707,
> 30008807&testName=tart&osName=Windows%20XP&server=graphs.mozilla.org
> 
> So I conclude that something about bug 918613 slowed UX down on XP.
> 
> Any idea what that might have been, Bas?

Not really, locally it strictly made things faster. However, there was a bug of something not drawing -without- 918613. Is it possible that this was -not- being drawn in TART? With the bug fixed, we'd now start drawing this (see bug 919656), where we previously weren't. Drawing stuff would certainly cause a performance 'regression' over not drawing stuff :P.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Bas has asked for TART profiles with and without this patch.

Builds are running, and I'll post links to profiles when they're done:

With 918613: https://tbpl.mozilla.org/?tree=Try&rev=c3f92a4a030b
Without: https://tbpl.mozilla.org/?tree=Try&rev=ea9679ef4356

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Win XP profiles:

With 918613: http://people.mozilla.org/~bgirard/cleopatra/#report=03235128025f354ff35785a2a72b07a3f7361e65
Without: http://people.mozilla.org/~bgirard/cleopatra/#report=c684d363c015693867c35975acb1ea70472a518f

Anything illuminating in there, Bas?

Comment 5

[Mike Conley (:mconley) (:⚙️)]

As requested, here's a profile for a TART run on a build from just before the Azure merge happened:

http://people.mozilla.org/~bgirard/cleopatra/#report=af501a59d65bae9fa6b58edb49dd96b0892f37c4

Comment 6

[Mike Conley (:mconley) (:⚙️)]

It looks like bug 921038 + bug 764299 will be enough to neutralize the Windows XP regression, so I think we can dial back the priority on this one. I'd still like to know what happened, though...

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Both of the bugs mentioned in comment #6 have been fixed now, and XP has been neutralized.

Comment 8

[Bas Schouten (:bas.schouten)]

I should still look into this a little more though, I too, would really like to know what happened here.