1310113 - 1.92 - 2.22% tp5o (windows7-32, windows8-64, windowsxp) regression on push 2eebd44ff2e99123d3673ccacd0aa634d3cb4b85 (Tue Oct 11 2016)

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Alison Shiue]

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

Regressions:

  2%  tp5o summary windows8-64 pgo e10s     246.83 -> 252.31
  2%  tp5o summary windows8-64 opt e10s     309.36 -> 315.75
  2%  tp5o summary windowsxp opt e10s       355.32 -> 362.25
  2%  tp5o summary windows7-32 opt e10s     350.15 -> 356.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=3654

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]

I did some retriggers, and here is the better zooming view:
https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bmozilla-inbound,8b1190942ae6284bfbb1886559bc55b04d859283,1,1%5D&series=%5Bmozilla-inbound,a3464628f6c8e4c4299f0fe6dccc7c7ac0524aed,1,1%5D&series=%5Bmozilla-inbound,f36b02ef070e73ca0f076fe8ab19586d2573d97c,1,1%5D&zoom=1476103596722.9028,1476205742311.9697,302.69662921348316,380&selected=%5Bmozilla-inbound,f36b02ef070e73ca0f076fe8ab19586d2573d97c,37587,37621847,1%5D

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

Comment 2

[Ting-Yu Chou [:ting] (away)]

That is for debug purpose, which does extra work to protect the memory that could be written to 0 unexpectedly. So the perf regression in network is expected.

The patch will be back out once we find out the code that corrupts the memory.

Comment 3

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

thanks for the quick reply :ting.  We will keep this open and follow up prior to the merge of Firefox 52 to Aurora to ensure this is backed out.  When this is backed out, will bug 1293501 be closed?

Comment 4

[Ting-Yu Chou [:ting] (away)]

(In reply to Joel Maher ( :jmaher) from comment #3)
> thanks for the quick reply :ting.  We will keep this open and follow up
> prior to the merge of Firefox 52 to Aurora to ensure this is backed out. 
> When this is backed out, will bug 1293501 be closed?

I am not in confident that it will be backed out before the merge, what I am sure is it will be backed out once if

1) the debug code captures the culprit, or
2) the crash is still happening, which the debug code doesn't help

Whether will bug 1293501 be closed or not depends on it's (1) or (2).

I'll leave a comment here when it's backed out.

Comment 5

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

:ting, in 1 more week we will be merging this code into Aurora- now that this code has been in nightly for 3 weeks, one would assume you have collected enough debug information, or have determined that this isn't working.  Can you indicate the plans to resolve this regression?

Comment 6

[Ting-Yu Chou [:ting] (away)]

Unfortunately I haven't seen an ACCESS_VIOLATION_WRITE crash with the address (0x...0c or 0x...fc) I expect from the patch. There was a crash report with the signature, but it happened outside of the readonly region which is not enough to determine whether the patch is working or not.

Considering the volume of the crash is quite different between Nightly and Aurora, I'd like to have the patch merge to Aurora and see how it goes. Will there be any concerns with this?

51.0a2: https://crash-stats.mozilla.com/signature/?product=Firefox&version=51.0a2&address=%24ff8&signature=ReleaseData&date=%3E%3D2016-08-08T01%3A27%3A16.000Z&date=%3C2016-11-08T01%3A27%3A16.000Z#graphs
52.0a1: https://crash-stats.mozilla.com/signature/?product=Firefox&version=52.0a1&address=%24ff8&signature=ReleaseData&date=%3E%3D2016-08-08T01%3A26%3A58.000Z&date=%3C2016-11-08T01%3A26%3A58.000Z#graphs
All:    https://crash-stats.mozilla.com/signature/?product=Firefox&address=%24ff8&signature=ReleaseData&date=%3E%3D2016-08-08T01%3A27%3A16.000Z&date=%3C2016-11-08T01%3A27%3A16.000Z#graphs

Comment 7

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #6)
> All:   
> https://crash-stats.mozilla.com/signature/
> ?product=Firefox&address=%24ff8&signature=ReleaseData&date=%3E%3D2016-08-
> 08T01%3A27%3A16.000Z&date=%3C2016-11-08T01%3A27%3A16.000Z#graphs

Note the volume of this crash, which is why I think it's worthy to merge to Aurora.

Comment 8

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

Personally I don't see a problem- I am including :ritu so Release Management is aware of this.

Comment 9

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

I see this was backed out and we have a performance improvement:
== Change summary for alert #4514 (as of December 13 2016 02:03 UTC) ==

Improvements:

  2%  tp5o summary windows8-64 pgo e10s     268.87 -> 263.3

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

Comment 10

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

did we back this out on Aurora as well?

Comment 11

[Ting-Yu Chou [:ting] (away)]

Not yet, is waiting for approval from release manager.

Comment 12

[Ting-Yu Chou [:ting] (away)]

It has been backed out on Aurora.