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)

52 Branch
defect
Not set
normal

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
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)
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?
(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.
Component: Untriaged → Networking: HTTP
Product: Firefox → Core
Assignee: nobody → janus926
Whiteboard: [necko-active]
: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)
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)
(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.
Personally I don't see a problem- I am including :ritu so Release Management is aware of this.
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
did we back this out on Aurora as well?
Not yet, is waiting for approval from release manager.
It has been backed out on Aurora.
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.