1091664 - 5% Android Ts Paint regression on fx-team (v.36) Oct 28 from push 44c6d392bbc8

Status: RESOLVED WONTFIX

(Testing :: Talos, defect)

Description

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

here is a regression in ts, paint on android:
http://graphs.mozilla.org/graph.html#tests=%5B%5B83,64,29%5D%5D&sel=1414061814256,1414666614256&displayrange=7&datatype=running

I did some retriggers on tbpl:
https://tbpl.mozilla.org/?tree=Fx-Team&fromchange=cf419403a2eb&tochange=8546940128b3&jobname=fx-team%20talos%20remote-tspaint

The values jump >3900 pretty consistently after this push:
https://tbpl.mozilla.org/?tree=Fx-Team&fromchange=cf419403a2eb&tochange=8546940128b3&jobname=fx-team%20talos%20remote-tspaint&rev=44c6d392bbc8

there are a handful of bugs in there, what wasn't backed out or test only changes is:
* bug 1073584
* bug 1075438
* bug 1059797

Comment 1

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

:bc, can you see if your autophone caught this regression?

:gaurav0x, can you look at your patch and see if this could be the cause
:paul, can you look at your patch and see if this could be the cause
:glandium, can you look at your patch and see if this could be the cause

Comment 2

[Patrick Brosset <:pbro>]

bug 1073584 is about devtools code only, and I don't think this code runs while the android paint test is executed.

Comment 3

[Bob Clary [:bc] (inactive)]

Attachment: autophone results and regression ranges

Yeah, Autophone caught two on Oct 28. One bug was backed out but the regression remained. Sorry I wasn't watching more closely.

Comment 4

[David Teller [:Yoric] - still alive but not very active]

That sounds surprising. Bug 1075438 basically removes a function that is unused. This requires a tiny bit of refactoring to OS.File.read, but that shouldn't change anything.

Comment 5

[Mike Hommey [:glandium]]

Can you get numbers on try with bug 1059797 backed out?

Comment 6

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

https://tbpl.mozilla.org/?tree=Try&rev=57de65370205

Comment 7

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

https://tbpl.mozilla.org/?tree=Try&rev=072a1940b7c8

Comment 8

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

the try push with bug 1059797 backed out didn't seem to reduce the test values.

Comment 9

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

this is now on Aurora!

Comment 10

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

this is now on Beta!

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Bob has a good split of the two regressions in comment 3.

First, a Throbber Start regression on Nexus S single core devices:
http://phonedash.mozilla.org/#/org.mozilla.fennec/throbberstart/local-blank/norejected/2014-10-27/2014-10-29/notcached/noerrorbars/standarderror/notry

(I have no idea how the cset could have regressed though)

Note that the Nexus S lines went from ~1000ms to ~1300ms in this regression. Those lines dropped back down to ~1120ms on Dec 5th, so we still have an active regression from the initial event.

The second regression is a Throbber Stop issue on all devices:
http://phonedash.mozilla.org/#/org.mozilla.fennec/throbberstop/local-blank/norejected/2014-10-27/2014-10-29/notcached/noerrorbars/standarderror/notry

This one is clearly pointing at bug 1059797. Joel tried a Ts run with the patch backed out, but I don't think Bob tried a backout on Autophone. Could we try that? 

That said, bug 1059797 is a fairly important bug fix, and we might just have to live with the issue, unless Snorp or Mike can see what might cause a regression in the patch and tweak it.

Comment 12

[Bob Clary [:bc] (inactive)]

The patch for bug 1059797 actually backs out cleanly: https://treeherder.mozilla.org/#/jobs?repo=try&revision=302761fdb079 Let's see what happens.

Comment 13

[Bob Clary [:bc] (inactive)]

Attachment: 1. Summary of try run results comparing inbound to inbound with patch reverted.

See http://phonedash.mozilla.org/#/org.mozilla.fennec/throbberstart/local-blank/norejected/2015-01-23/2015-01-24/notcached/errorbars/standarderror/try

The first measurement is a mozilla-inbound build from 1/23 with no additional patches. The second measurement is a mozilla-inbound build from 1/24 with the patch reverted. Change the selects to see the various measurements described in the attachment.

The results aren't 100% consistent across the devices or first run vs. second run, but it does appear that reverting the patch improves performance over all. Part of the problem in the comparison is the two builds are based on mozilla-inbound from different days.

Autophone is behind on builds at the moment but I will submit two try builds this morning from the same revision: one with the patch and one with it reverted. I'll let you know when it completes.

Comment 14

[James Willcox (:snorp) ([e-mail]) (he/him)]

This is pretty bizarre. That patch should only be doing an allocation that was going to happen slightly later anyway.

We do have a better understanding on that bug, however, so we can probably come up with a different fix.

Comment 15

[Mike Hommey [:glandium]]

As I read the current data on phonedash, the builds with bug 1059797 backed out are actually slower than the builds without.

Comment 16

[Bob Clary [:bc] (inactive)]

Attachment: 2. Summary of try run results comparing inbound to inbound with patch reverted.

The results are again mixed with the two runs I did yesterday off of the same revision of inbound.

Comment 17

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

this has shipped, any issues with closing this as wontfix?

Comment 18

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Joel Maher (:jmaher) from comment #17)
> this has shipped, any issues with closing this as wontfix?

OK with me