1321749 - 2 - 2.91% damp (linux64, osx-10-10, windows7-32, windows8-64) regression on push 30baf39ed89ff9c3efe4ffc1416835e9ed790b50 (Thu Dec 1 2016)

Status: RESOLVED WONTFIX

(DevTools :: Netmonitor, defect)

Description

[Alison Shiue]

Talos has detected a Firefox performance regression from push 30baf39ed89ff9c3efe4ffc1416835e9ed790b50. As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

  3%  damp summary windows7-32 pgo e10s     228.53 -> 235.19
  2%  damp summary windows8-64 pgo e10s     223.2 -> 228.69
  2%  damp summary linux64 pgo e10s         247.11 -> 253.07
  2%  damp summary windows8-64 opt e10s     274.11 -> 279.75
  2%  damp summary osx-10-10 opt e10s       303.55 -> 309.62


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

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

[Alison Shiue]

After doing some retriggers, this issue might be caused by some of the following patches:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ff48df015e576a28208bb3c09d553f909fcbb3d1&tochange=30baf39ed89ff9c3efe4ffc1416835e9ed790b50

Hi Jarda, as you are the patch author, can you take a look at this and determine what is the root cause? Thanks!

Comment 2

[Jarda Snajdr [:jsnajdr]]

Hi Alison, yes, the regression is caused by bug 1309866. We were aware of that before landing, and after a long discussion, we decided to land the patch despite the regression. We plan to get the performance back to the previous level with a series of followup optimization patches.

Comment 4

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

should we leave this bug open or resolve it?  I am not sure if we plan to fix the perf issues as part of other work, or specific focus on perf to resolve this bug.

Comment 5

[Jarda Snajdr [:jsnajdr]]

Hi Joel, let's keep this open - I'm investigating the performance issues right now and I plan to get the performance numbers back to the previous level (or better) before the 54 merge date (Jan 23).

Comment 6

[Andrew Overholt [:overholt]]

(In reply to Jarda Snajdr [:jsnajdr] from comment #5)
> Hi Joel, let's keep this open - I'm investigating the performance issues
> right now and I plan to get the performance numbers back to the previous
> level (or better) before the 54 merge date (Jan 23).

Do you plan to uplift that work to 53 or should we mark this as wontfix/fix-optional for 53? Thanks.

Comment 7

[Jarda Snajdr [:jsnajdr]]

(In reply to Andrew Overholt [:overholt] from comment #6)
> Do you plan to uplift that work to 53 or should we mark this as
> wontfix/fix-optional for 53? Thanks.

Our goal is to have 53 in perfect condition without any regressions. If that means uplift after merge day, then we'll uplift.

Comment 9

[Andrew Overholt [:overholt]]

:thumbsup: is what I meant to say :)

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Any word on uplifts for this to 53?  Where does it stand?

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Let's ask Honza as well, since he is leading Netmonitor efforts and reviewed the bug in question.

Comment 12

[Jan Honza Odvarko [:Honza] (always need-info? me)]

We are very close to have XUL fully removed from the panel but, some remaining stuff (e.g. <xul:splitter>) still needs to be replaced by React components. After this is done we can incorporate react-virtualized and improve the performance to fix the regression. So, this bug still needs to be open for now.

Honza

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Still happy to take patches for aurora 53.

Comment 14

[Ricky Chien [:rickychien] (inactive)]

According to Jarda's comment from https://bugzilla.mozilla.org/show_bug.cgi?id=1308441#c116:

> The 'requestsFinished' timing was added in bug 1328553. Before that, the
> DAMP timings were not very well-defined and were giving random results.
> Comparing current DAMP results with bug 1321749 is longer meaningful today,
> before the 'reload' and 'close' timings are done slightly differently. I
> think that bug 1321749 can be closed and solved.

We agree with that this DAMP regression is no longer meaningful after bug 1328553 is landed. Let's close this issue!

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Looks like this was fixed in bug 1328553. Thanks!