Closed Bug 1300046 Opened 8 years ago Closed 8 years ago

2.11 - 2.2% tabpaint / tpaint (windows8-64) regression on push 4f9433d3d361884c844a14cd52263c1653f2d83f (Thu Sep 1 2016)

Categories

(Firefox :: Untriaged, defect)

51 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
firefox51 --- fixed

People

(Reporter: ashiue, Unassigned)

References

Details

(Keywords: perf, regression, talos-regression)

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

Regressions:

  2%  tpaint windows8-64 opt               265.37 -> 271.21
  2%  tabpaint summary windows8-64 opt     84.46 -> 86.24


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

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
After doing some retriggers, this issue might be caused by one of following patches:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a3385e09f0c0193803b26afb5ff1583603fc53f7&tochange=4f9433d3d361884c844a14cd52263c1653f2d83f

Hi Shu-yu, as you are the patch author, can you take a look at this and determine what is the root cause? Thanks!
Blocks: 1291351, 1297706
Flags: needinfo?(shu)
Jan's volunteered to take over that bug even though I authored the original patches. Redirecting ni.
Flags: needinfo?(shu) → needinfo?(jdemooij)
These patches make us use the lazy parser for more code. It's really hard to say why it would regress these tests - it's possible we can now lazy-parse more JS used by these tests, and if a lot of this lazily-parsed code is actually used, it's a bit slower to syntax parse and then full parse. It might also be possible we relazify code, so we full-parse more than once.

In general, lazy parsing is a pretty big memory and performance win, and bug 1297706 makes lazy parsing work for more JS features.

Locally on OS X, I don't see a regression for tabpaint and tpaint (it might be a small win even but the numbers are very noisy).

Joel, what do you think? It's really hard to track down a regression that's < 2.5% and with noisy data, and only on some platforms. The changes in bug 1297706 are important though and will definitely improve things elsewhere...
Flags: needinfo?(jdemooij) → needinfo?(jmaher)
I don't think there is a problem with leaving this in and accepting it.  The main goal of the bug is to raise awareness to developers of interest and find the root cause.  If we can do something smarter, lets do it.  If we feel this is one piece of many and it just so happens to be this has a regression where other pieces have known perf wins (regardless if Talos shows a win or not), I am fine accepting it.

overall on tpaint for win8, we have a lot of wins in the last 2 months.  For tabpaint on win8 there are a bunch of ups and downs, overall in the last couple months probably a slight regression.  In fact this effectively reverses the win for tpaint seen in https://treeherder.mozilla.org/perf.html#/alerts?id=2774, coming from bug 1022601.

If you determine that we should accept this regression, please feel free to mark this as resolved/fixed :)
Flags: needinfo?(jmaher)
:jandem, is there work to do here, or is it most likely a resolved/wontfix?
Flags: needinfo?(jdemooij)
Thanks :)
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(jdemooij)
Resolution: --- → FIXED
Per comment #4, mark 51 as fixed.
You need to log in before you can comment on or make changes to this bug.