1367351 - 9.33 - 78.05% cart / tsvgx (linux64) regression on push 830c06ded9bb474004bbf8aaf9d8f9b44d811147 (Tue May 23 2017)

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Ionuț Goldan [:igoldan]]

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=830c06ded9bb474004bbf8aaf9d8f9b44d811147

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

 78%  cart summary linux64 opt e10s     29.74 -> 52.95
  9%  tsvgx summary linux64 opt e10s    433.44 -> 473.88


You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=6788

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the Talos jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://wiki.mozilla.org/Buildbot/Talos/Tests

For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/Buildbot/Talos/Running

*** Please let us know your plans within 3 business days, or the offending patch(es) will be backed out! ***

Our wiki page outlines the common responses and expectations: https://wiki.mozilla.org/Buildbot/Talos/RegressionBugsHandling

Comment 1

[Ionuț Goldan [:igoldan]]

Could you confirm this regression? Looks like it only happens on linux64.

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

I don't see how our change may cause a regression in tsvgx or cart.

tsvgx loads svg pages, it's unlikely to load those by doing a search in the location bar, right? If this test is affected in any way, sounds like it may be a bogus test.

cart is measuring animation of the customization panel... Again, I can't think of a reason why enabling search suggestions in the location bar may have anything to do with this, unless the test is typing something in the locationbar, that would be dumb.

Though, I have a plausible theory, that these tests are opening new tabs and as a consequence focusing the locationbar and showing the onboarding notification.

I suspect talos doesn't respect prefs_general.js right?
It seems to have a different place for prefs that is:
http://searchfox.org/mozilla-central/source/testing/talos/talos/base_profile/prefs.js

If so, we should probably add to it
user_pref("browser.urlbar.userMadeSearchSuggestionsChoice", true);

Comment 3

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

Ionut, add it here:
https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/config.py?q=path%3Atesting%2Ftalos%2Ftalos%2Fconfig.py&redirect_type=single#60

I assume a try push will prove this quickly!

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

let's see https://treeherder.mozilla.org/#/jobs?repo=try&revision=710af955076416c5e901d965235fc3122d011dd0

Comment 5

[Ionuț Goldan [:igoldan]]

Didn't see comment 4, and so I also pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cadf43431f9f71e78edefce791060e1e49360011

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

sounds good, feel free to cancel my builds, or just use them as an additional baseline, we have a good number of linux64 builders, and I actually didn't retrigger multiple Talos runs...

Comment 7

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

I see:
cart summary opt e10s: 29.7
tsvgx summary opt e10s: 431.69
These looks like the previous values... I only have 2 datapoints, Ionut may have more.

Feel free to make/review a patch here, I'm fine with that, I don't have further time for today :(

Comment 8

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

Ionut, your base revision was prior to this code landing, so the good numbers are sort of false.  Can you push with a recent revision?

Comment 9

[Ionuț Goldan [:igoldan]]

Sorry about that miss. Strange how I've picked the wrong changeset... Probably I didn't properly save the hash in clipboard.
I'm pushing the change again.

Comment 10

[Ionuț Goldan [:igoldan]]

::mak :jmaher This is the second push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=033e95ec7f8ac23d68f8b815cae3d926251dc96a

Comment 11

[Ionuț Goldan [:igoldan]]

Results came in, and this is a comparison, againts the last changeset, before this alerted regression:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=autoland&originalRevision=2fde169875b3&newProject=try&newRevision=033e95ec7f8a&framework=1&filter=cart%20linux&showOnlyImportant=0

None of the cart and tsvgx regressions seem to appear, after calibrating the Talos tests.
:jmaher Can you confirm this so to mark bug 1367351 as invalid?

Comment 12

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #11)
> so to mark bug 1367351 as invalid?

By invalid you mean we should land your patch in central, right? Do you mind attaching it with a proper commit message for review?

Comment 13

[Ionuț Goldan [:igoldan]]

(In reply to Marco Bonardo [::mak] from comment #12)

> By invalid you mean we should land your patch in central, right?
  Yes.
> Do you mind attaching it with a proper commit message for review?
  I actually encourage you to do that.

But before doing this, I need to coordinate with Joel.

Comment 14

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: Bug 1367351 - Avoid the search suggestions onboarding in Talos tests.

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

Comment 15

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

Comment on attachment 8871197 [details]
Bug 1367351 - Avoid the search suggestions onboarding in Talos tests.

https://reviewboard.mozilla.org/r/142688/#review146354

Comment 16

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

thanks for getting this verified on Try Ionut!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05eaf5ceeba3
Avoid the search suggestions onboarding in Talos tests. r=jmaher

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/05eaf5ceeba3

Comment 19

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

here are some improvements related to this:
== Change summary for alert #6858 (as of May 25 2017 08:59 UTC) ==

Improvements:

 32%  cart summary linux64 opt e10s     42.93 -> 29.35
  8%  tsvgx summary linux64 opt e10s    465.22 -> 429.88
  2%  tp5o_webext Main_RSS linux64 opt e10s186,701,394.26 -> 182,171,839.26
  2%  tp5o Main_RSS linux64 opt e10s    179,690,743.71 -> 175,542,737.99

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=6858