Talos has detected a Firefox performance regression from push: https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=f4ae445c27170b47c330b7e5091063dc43d408ca As author of one of the patches included in that push, we need your help to address this regression. Regressions: 5% tp5o_webext opt e10s stylo 211.61 -> 221.36 4% tp5o_webext opt e10s stylo 190.40 -> 198.42 4% tp5o_webext pgo e10s stylo 174.53 -> 181.77 4% tp5o_webext opt e10s stylo 209.39 -> 217.94 4% tp5o_webext pgo e10s stylo 186.77 -> 194.15 4% tp5o_webext opt e10s stylo 196.08 -> 203.67 3% tp5o_webext pgo e10s stylo 177.64 -> 183.79 3% tp5o_webext opt e10s stylo 193.32 -> 199.70 You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=18233 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/Performance_sheriffing/Talos/Tests For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/Performance_sheriffing/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/Performance_sheriffing/Talos/RegressionBugsHandling

(In reply to Florin Strugariu [:Bebe] from comment #0) > Regressions: > > 5% tp5o_webext opt e10s stylo 211.61 -> 221.36 > 4% tp5o_webext opt e10s stylo 190.40 -> 198.42 > 4% tp5o_webext pgo e10s stylo 174.53 -> 181.77 > 4% tp5o_webext opt e10s stylo 209.39 -> 217.94 > 4% tp5o_webext pgo e10s stylo 186.77 -> 194.15 > 4% tp5o_webext opt e10s stylo 196.08 -> 203.67 > 3% tp5o_webext pgo e10s stylo 177.64 -> 183.79 > 3% tp5o_webext opt e10s stylo 193.32 -> 199.70 Because of a reporting flaw, the regressions don't mention the platforms. Here are the complete results: Regressions: 5% tp5o_webext linux64-qr opt e10s stylo 211.61 -> 221.36 4% tp5o_webext windows10-64-qr opt e10s stylo 190.40 -> 198.42 4% tp5o_webext windows7-32 pgo e10s stylo 174.53 -> 181.77 4% tp5o_webext linux64 opt e10s stylo 209.39 -> 217.94 4% tp5o_webext linux64 pgo e10s stylo 186.77 -> 194.15 4% tp5o_webext windows10-64 opt e10s stylo 196.08 -> 203.67 3% tp5o_webext windows10-64 pgo e10s stylo 177.64 -> 183.79 3% tp5o_webext windows7-32 opt e10s stylo 193.32 -> 199.70

I'll try to post the Gecko profiles.

Unfortunately, I didn't have any luck. Looks like it's the same problem experienced here [1] and stated in bug 1514257. [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1514257#c0

Does this mean the reporting is inaccurate and not a result of the changes in bug 1437258? We were looking at it yesterday, trying to even get tp5o_webext to run (non obvious). It's hard to imagine the changes in that patch causing this level of regression.

Here's a recent run with some minor changes, but even without those changes, the code paths here dont make sense to have caused that much regression. https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=85cf888dafaa8c1a8373729b0124fce6612947d9

I believe I have found the cause of this regression, and I have a potential solution. I have to check it with :mixedpuppy, but it should be able to be fixed by the end of 2018-12-21.

(In reply to Shane Caraveo (:mixedpuppy) from comment #4) > Does this mean the reporting is inaccurate and not a result of the changes > in bug 1437258? We were looking at it yesterday, trying to even get > tp5o_webext to run (non obvious). It's hard to imagine the changes in that > patch causing this level of regression. No, the reporting is accurate. It's just that it was incomplete.

This patch includes the following changes: - move into the API schema wrappers the additional checks needed (in the child process) to ensure that we throw a validation error when a webRequest blocking listener is registered by an extension that doesn't have the required webRequestBlocking permission: - define a new webRequestBlockingPermissionRequired postprocessor - add to web_request.json API schema the `"postprocess": "webRequestBlockingPermissionRequired"` property to all the webRequest options types that allows "blocking" as a valid string value (currently OnBeforeRequestOptions, OnBeforeSendHeadersOptions, OnHeadersReceivedOptions and OnAuthRequiredOptions) - add back an additional check for the webRequestBlocking permission in parent/ext-webRequest.js (just in case a new webRequest event is added in the future and we forget to add the postprocess property, so that we still ensure that an extension without the webRequestBlocking permission is not able to register any blocking listener) - tweak the test_no_webRequestBlocking_error test case to use browser.test.assertThrows to explicitly check that the error is thrown and "catchable" by the extension code, and not just logged in the console messages, and extend it to run on all the webRequest API events that can be blocking.

As I've been discussing over IRC with :tossj, the patch I just attached seems to give talos results similar to the patch that :tossj was preparing, in :tossj's patch fire.sync was being changed to fire.raw, and I think that @tossj analysis was right and fire.sync was likely the part that was introducing a good chunk of the performance regression detected by the talos (which makes sense because fire.sync clones the data into the extension context, which should also be unnecessary because that data is coming from a parent event and so it has been already cloned in the extension context when the StructuredCloneHolder has been deserialized) Nevertheless, we agreed (@tossj and I) that we both liked this alternative strategy a bit more than the one we are currently using and so I proceeded to attach the patch. @mixedpuppy as a blocking reviewer on the patch, and @tossj as an additional reviewer (e.g. to double-check that this version is still providing the intended behavior in the exact same way of the current version).

:rpl, I don't have the permissions to accept the phabricator revision, but it is working as intended; the error message is displaying on the extension debugging console when an extension with "blocking" doesn't have the webRequestBlocking permission.

At the following url the results from running talos tests on the attached patch and compare its results against the results on mozilla-central (over the last 2 days): https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=ff4a893467987e6d7943550c4c7a260e5c2273c6&framework=1&filter=webext&showOnlyComparable=1&showOnlyImportant=1&selectedTimeRange=172800

The related push to try also looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff4a893467987e6d7943550c4c7a260e5c2273c6

(In reply to tossj from comment #10) > :rpl, I don't have the permissions to accept the phabricator revision, but > it is working as intended; the error message is displaying on the extension > debugging console when an extension with "blocking" doesn't have the > webRequestBlocking permission. Thanks for double-checking it for me (and also for digging into which part of the original patch may have contributed to the regression, that wasn't an easy task ;-)).

Pushed by luca.greco@alcacoop.it: https://hg.mozilla.org/integration/autoland/rev/c9413e3a5702 Move to API schema wrappers the webRequestBlocking permission checks for webRequest blocking listeners. r=mixedpuppy

I see an improvement: == Change summary for alert #18383 (as of Thu, 20 Dec 2018 19:51:51 GMT) == Improvements: 3% tp5o_webext windows7-32 opt e10s stylo 201.24 -> 194.43 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=18383 I assume this is spread across all platforms, I saw windows10 alert on inbound, just not on autoland.

here are the other platforms: == Change summary for alert #18377 (as of Thu, 20 Dec 2018 18:31:18 GMT) == Improvements: 5% tp5o_webext linux64-qr opt e10s stylo 221.28 -> 210.75 4% tp5o_webext windows7-32 pgo e10s stylo 183.35 -> 175.33 4% tp5o_webext linux64 pgo e10s stylo 194.43 -> 186.11 4% tp5o_webext linux64 opt e10s stylo 217.45 -> 209.13 4% tp5o_webext windows10-64 pgo e10s stylo 183.76 -> 177.18 3% tp5o_webext windows10-64 opt e10s stylo 203.55 -> 196.61 3% tp5o_webext windows10-64-qr opt e10s stylo 198.35 -> 192.17 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=18377