Closed Bug 1091664 Opened 6 years ago Closed 5 years ago

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


(Testing :: Talos, defect)

Not set


(Not tracked)



(Reporter: jmaher, Unassigned)



(Keywords: perf, regression, Whiteboard: [talos_regression])


(3 files)

: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
Flags: needinfo?(paul)
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(gaurav_rai)
Flags: needinfo?(bclary)
bug 1073584 is about devtools code only, and I don't think this code runs while the android paint test is executed.
Flags: needinfo?(paul)
Yeah, Autophone caught two on Oct 28. One bug was backed out but the regression remained. Sorry I wasn't watching more closely.
Flags: needinfo?(bclary)
That sounds surprising. Bug 1075438 basically removes a function that is unused. This requires a tiny bit of refactoring to, but that shouldn't change anything.
Can you get numbers on try with bug 1059797 backed out?
Flags: needinfo?(mh+mozilla)
the try push with bug 1059797 backed out didn't seem to reduce the test values.
this is now on Aurora!
this is now on Beta!
Flags: needinfo?(gaurav_rai)
Bob has a good split of the two regressions in comment 3.

First, a Throbber Start regression on Nexus S single core devices:

(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:

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.
Flags: needinfo?(snorp)
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(bob)
The patch for bug 1059797 actually backs out cleanly: Let's see what happens.

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.
Flags: needinfo?(bob)
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.
Flags: needinfo?(snorp)
As I read the current data on phonedash, the builds with bug 1059797 backed out are actually slower than the builds without.
Flags: needinfo?(mh+mozilla)
The results are again mixed with the two runs I did yesterday off of the same revision of inbound.
this has shipped, any issues with closing this as wontfix?
(In reply to Joel Maher (:jmaher) from comment #17)
> this has shipped, any issues with closing this as wontfix?

OK with me
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.