1489072 - 1.7 - 2.59% ts_paint / ts_paint_webext (linux64) regression on push f4400f4e6b8500d2a7ebaacbaa1a361f18c9feac (Tue Sep 4 2018)

Status: VERIFIED FIXED

(Firefox :: PDF Viewer, defect, P1)

Description

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

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=f4400f4e6b8500d2a7ebaacbaa1a361f18c9feac

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

Regressions:

  3%  ts_paint_webext linux64 opt e10s stylo     194.96 -> 200.00
  2%  ts_paint linux64 opt e10s stylo            196.00 -> 199.33


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

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

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

:kmag, your change in bug 1481859 caused a talos performance regressions to startup times, can you please look into this?

Comment 2

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

profile before:
https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FUyx7h-u_SNCQ1j2XWj2f5Q%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_ts_paint_webext.zip

profile after:
https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FfN6_1TlRQuy-QwS5jZqLvw%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_ts_paint_webext.zip

Comment 3

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

:kmag, can you give an update here so we know if you are working on this and when we can expect a fix?

Comment 4

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

:kmag, I have not heard anything here, should we back out?  We do have a policy within 3 business days to comment on the bug and explain the regression or an estimate and way to track when the work will be done.

Comment 5

[Kris Maglione [:kmag]]

I'm working on a fix. I should have one by the end of the week.

Comment 6

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

Thanks!

Comment 7

[Kris Maglione [:kmag]]

Attachment: Bug 1489072: Fix startup perf regression in pdf.js init. r=bdahl

Comment 8

[David Durst [:ddurst]]

Setting to P1 and CCing Pascal because of the relationship to bug 1481859, which is P1 and tracking 63.

Comment 9

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

:kmag, did you intend to ask :bdahl for a review in this bug?

Comment 10

[David Durst [:ddurst]]

(In reply to Joel Maher ( :jmaher ) (UTC-4) from comment #9)
> :kmag, did you intend to ask :bdahl for a review in this bug?

It was done via Phabricator. Unfortunately, this is no longer apparent in bugzilla.

Comment 11

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

got it; so many sources of truth these days.

Comment 12

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/70b36ec329e7ff9139c098f0a029243a57df61be
Bug 1489072: Fix startup perf regression in pdf.js init. r=bdahl

Comment 13

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/70b36ec329e7

Comment 14

[Ionuț Goldan [:igoldan]]

The fix seems to have canceled the regressions above:

== Change summary for alert #16579 (as of Mon, 08 Oct 2018 18:57:37 GMT) ==

Improvements:

  2%  ts_paint_webext windows7-32 pgo e10s stylo     317.42 -> 310.67

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

Comment 15

[Daniel Varga [:dvarga]]

Please request beta approval for this. 
As https://bugzilla.mozilla.org/show_bug.cgi?id=1481859 depends on this bug.

Comment 16

[Brendan Dahl [:bdahl]]

Comment on attachment 9014157 [details]
Bug 1489072: Fix startup perf regression in pdf.js init. r=bdahl

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1481859

User impact if declined: Startup performance regression.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: Bug 1481859

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Worst case, something goes wrong with the cached state and the PDF viewer is either stuck enabled/disabled until a settings change. Alternatively, we land the other patch, but suffer a performance regression.

String changes made/needed:

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9014157 [details]
Bug 1489072: Fix startup perf regression in pdf.js init. r=bdahl

Blocks bug 1481859, approved for 63.0b14.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/91955baf362b