Closed
Bug 1310113
Opened 8 years ago
Closed 8 years ago
1.92 - 2.22% tp5o (windows7-32, windows8-64, windowsxp) regression on push 2eebd44ff2e99123d3673ccacd0aa634d3cb4b85 (Tue Oct 11 2016)
Categories
(Core :: Networking: HTTP, defect)
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox49 | --- | unaffected |
firefox50 | --- | unaffected |
firefox51 | --- | unaffected |
firefox52 | --- | fixed |
firefox53 | --- | fixed |
People
(Reporter: ashiue, Assigned: ting)
References
Details
(Keywords: perf, regression, talos-regression, Whiteboard: [necko-active])
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
Reporter | ||
Comment 1•8 years ago
|
||
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!
Assignee | ||
Comment 2•8 years ago
|
||
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.
Flags: needinfo?(janus926)
Comment 3•8 years ago
|
||
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?
Assignee | ||
Comment 4•8 years ago
|
||
(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.
Updated•8 years ago
|
status-firefox49:
--- → unaffected
status-firefox50:
--- → unaffected
status-firefox51:
--- → unaffected
Updated•8 years ago
|
Component: Untriaged → Networking: HTTP
Product: Firefox → Core
Comment 5•8 years ago
|
||
: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?
Flags: needinfo?(janus926)
Assignee | ||
Comment 6•8 years ago
|
||
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
Flags: needinfo?(janus926)
Assignee | ||
Comment 7•8 years ago
|
||
(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•8 years ago
|
||
Personally I don't see a problem- I am including :ritu so Release Management is aware of this.
Updated•8 years ago
|
status-firefox53:
--- → affected
Comment 9•8 years ago
|
||
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•8 years ago
|
||
did we back this out on Aurora as well?
Assignee | ||
Comment 11•8 years ago
|
||
Not yet, is waiting for approval from release manager.
Assignee | ||
Comment 12•8 years ago
|
||
It has been backed out on Aurora.
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•