Talos has detected a Firefox performance regression from push ea340a9879aace83eac47e95cbdb3c2eeeaf5fd9. As author of one of the patches included in that push, we need your help to address this regression. Regressions: 3% tresize windows7-32 opt e10s 12.5 -> 12.94 3% tresize osx-10-10 opt e10s 29.36 -> 30.3 You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=4602 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 the following patches: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b00f52ee67ba46fc48117304a161c5ff91cc16a7&tochange=ea340a9879aace83eac47e95cbdb3c2eeeaf5fd9 Hi cjku, as you are the patch author, can you take a look at this and determine what is the root cause? Thanks!

After Bug 1318266 landed, we may paint clip-path onto mask layer under certain condition, and then use gpu to compose the mask. The problem is we should not hit that condition while running Talos. I will check

At [1], ContainerState will build a mask layer of nsDisplayMask::GetLayerState return LAYER_INACTIVE and mManager->IsWidgetLayerManager() return false. Depend on the platform, mManager->IsWidgetLayerManager() returns true on linux and false on windows. So we should return LAYER_SVG_EFFECTS to make sure not creating mask layer. [1] https://hg.mozilla.org/mozilla-central/file/cd0548c3fe1c/layout/painting/FrameLayerBuilder.cpp#l4157

Local test After apply this patch: PERFHERDER_DATA: {"framework": {"name": "talos"}, "suites": [{"subtests": [{"replicates": [13.54938333333332, 13.03389999999996, 13.673783333333317, 13.506616666666652, 13.09853333333334, 12.833383333333323, 13.409750000000006, 13.090516666666632, 12.90366666666667, 12.976433333333347, 13.289233333333335, 13.615933333333343, 13.220416666666674, 12.56659999999999, 13.107599999999971, 12.960633333333297, 12.789850000000007, 13.408183333333357, 13.016666666666701, 13.289283333333325], "name": "tresize", "value": 13.090516666666632}], "extraOptions": ["e10s"], "name": "tresize"}]} Before apply this patch PERFHERDER_DATA: {"framework": {"name": "talos"}, "suites": [{"subtests": [{"replicates": [14.427783333333336, 14.302716666666614, 14.385716666666662, 14.481933333333284, 14.290183333333353, 14.848949999999974, 14.220983333333345, 14.786300000000017, 14.26963333333334, 14.419583333333339, 14.444283333333281, 14.547449999999978, 14.367716666666674, 14.240966666666658, 14.440066666666661, 14.27973333333334, 14.215716666666694, 14.396816666666687, 14.766566666666673, 14.078916666666657], "name": "tresize", "value": 14.396816666666687}], "extraOptions": ["e10s"], "name": "tresize"}]}

Comment on attachment 8821477 [details] Bug 1325550 - Add default layer state value as a parameter of RequiredLayerStateForChildren. https://reviewboard.mozilla.org/r/100786/#review101262 Ok, this is fine as a short-term workaround. However, LAYER_INACTIVE means that we use a mask layer in the temporary BasicLayerManager that's created for the inactive display item. So we will apply the mask on the main thread during painting, and not on the Compositor. Applying mask layers in BasicLayerManager does very similar things to what nsSVGIntegrationUtils::PaintMaskAndClipPath does. There's not a good reason that it should be any slower. We should find out why it's slower and fix it, so that we can get rid of LAYER_SVG_EFFECTS for masks entirely.

This testcase might be useful for profiling.

Filed Bug 1325590.

Pushed by cku@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2df6e9896fbe Add default layer state value as a parameter of RequiredLayerStateForChildren. r=mstange

thanks for fixing this so quickly- I have verified on the graphs that we have improvements!

The patch I check in here is not right 1. In RequiredLayerStateForChildren, if we set the default value of result as LAYER_SVG_EFFECT, this function wil always return LAYER_SVG_EFFECT, so we will never paint on mask layer again. But because of it, this pref regression seem to be fixed. 2. I will temporary disable paint clip-path on mask layer, roll back change in RequiredLayerStateForChildren, close this bug and bug 1325849, reopen bug 1318266. Figure out perf regression of talos:tresize, fix it and then turn on paint clip-path onto mask layer

https://hg.mozilla.org/integration/mozilla-inbound/rev/ab28e51be735e2cc72ffc77d998e5c7d9da22f5d Bug 1325550 - Rollback change and temporary disbale paint clip-path onto mask layer. r=me

(In reply to C.J. Ku[:cjku](UTC+8) from comment #15) > The patch I check in here is not right > 1. In RequiredLayerStateForChildren, if we set the default value of result > as LAYER_SVG_EFFECT, this function wil always return LAYER_SVG_EFFECT, so we > will never paint on mask layer again. But because of it, this pref > regression seem to be fixed. No always, but most of times. For exp, the if statement at [1][2] will always be false, since LAYER_SVG_EFFECT(5) > LAYER_ACTIVE(2) [1] http://searchfox.org/mozilla-central/source/layout/painting/nsDisplayList.cpp#4600 [2] http://searchfox.org/mozilla-central/source/layout/painting/nsDisplayList.cpp#4604 > 2. I will temporary disable paint clip-path on mask layer, roll back change > in RequiredLayerStateForChildren, close this bug and bug 1325849, reopen bug > 1318266. Figure out perf regression of talos:tresize, fix it and then turn > on paint clip-path onto mask layer