Talos has detected a Firefox performance regression from push 2f96ca59484d. As author of one of the patches included in that push, we need your help to address this regression. Regressions: 3% kraken summary osx-10-10 opt 1478.84 -> 1517.86 You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=4477 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

here is a link to the compare view: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=mozilla-inbound&originalRevision=28d3c72eaecdc719fa4a01d5bb2d345b216070a9&newProject=mozilla-inbound&newRevision=2f96ca59484d38b4b6d2f3aca283001bc7f01029&originalSignature=2fbb12cea9acbb6ef8452323642a57a9d993ce79&newSignature=2fbb12cea9acbb6ef8452323642a57a9d993ce79&framework=1 the regression is in: kraken imaging-gaussian-blur :h4writer, I see you wrote the patches that caused the regression, is this expected or is there any work we can do to fix the regression?

oh, I see 1322724 from https://treeherder.mozilla.org/perf.html#/alerts?id=4482. Possibly this is fixed?

At least on AWFY, the patch from bug 1322724 didn't fix most of the regressions, including the one pointed out in this bug.

Status: Bug 1322724 fixed a little bit of the regression, but there seems to be more left looking at the improvements after the fix. I'll investigate this week. Currently traveling. I should be able to look at it Wednesday.

I traced the kraken-gaussian regression to the "FoldTests" not applying cleanly anymore. An extra empty block is added in http://searchfox.org/mozilla-central/source/js/src/jit/IonBuilder.cpp#2832 which "FoldTests" doesn't recognizes. I could have fixed that logic to support an extra empty block in the true/false path, but I didn't see how that would scale. I've updated that logic before and there are potential other places that depend on it. Therefore I though it would be better to introduce a step that would remove such extra empty blocks. 1) No need to update the FoldTest logic to add support for empty blocks in between 2) Decreases the amount of blocks in a graph. Which means all other passes need to iterate less blocks. We could potentially call this again after GVN to remove even more of those blocks. (We often have a chain of empty blocks at the end of MIR) 3) Having these empty blocks could disable some optimizations because we didn't add code to also match empty blocks. 4) The IonBuilder CFG split caused some other places with extra empty blocks for convenience. This removes those too. 5) We can increase this logic to merge blocks with non-effectfull instructions. A next step would be to merge the "MFilterTypeSet" in the next block and have only one block if possible. This fixed the regression locally. I even saw an improvement, but I don't think this will translate in a awfy gaussian win.

Comment on attachment 8818922 [details] [diff] [review] Patch Review of attachment 8818922 [details] [diff] [review]: ----------------------------------------------------------------- Makes sense.

Pushed by hv1989@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0703ad673049 IonMonkey - Remove empty blocks, r=jandem

The improvement shows on https://treeherder.mozilla.org/perf.html#/alerts?id=4560, thanks.