Talos has detected a Firefox performance regression from push 03f394bbe821227153b23697825099022b601e8b. As author of one of the patches included in that push, we need your help to address this regression. Regressions: 28% bloom_basic http: osx-10-10 opt e10s 551.42 -> 705.01 27% bloom_basic_ref http: osx-10-10 opt e10s 550.62 -> 699.81 You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=6090 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

Does this perhaps match the regressions seen on cross-mac builds?

If this ever goes to the backout route, please only backout 7e67993e7c5b, not all three changesets from the push.

(In reply to (VAC until May 4) Mike Hommey [:glandium] from comment #1) > Does this perhaps match the regressions seen on cross-mac builds? That's bug 1338651, and no, doesn't look like it, unfortunately.

Note there are actually 3 pushes, for 2 bugs in the range in the alert. https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ad85c3fedd6750b955af6ef6219fa411391ae8f1&tochange=03f394bbe821227153b23697825099022b601e8b Could as much be bug 1344357.

bummer that only this test is affected by the compiler changes here- if others were that would be a good hint at what issues we have with cross-compiled builds. as a note, the range was larger until we backfilled the data, that is how we narrowed down on bug 1356927. Would a clobber be related at all? :igoldan, can you look at a try run with the current tree, then with the tree + backout of 7e67993e7c5b? That would help determine if we are looking at just that revision. :ted, as :glandium is on holiday for 2 weeks (and we should respect that), could you help look into this? Maybe you could help find someone if you don't have time.

:jmaher Sure thing.

These tests seem to just add a bunch of CSS rules and call getComputedStyle, so the regression must be either in DOM or style code: https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/tests/perf-reftest/bloom-basic.html https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/tests/perf-reftest/bloom-basic-ref.html I'm not really sure what it means that both tests regressed by about the same amount, I guess that the bloom filter (which is the point of the test) is still working as designed, but something else got slower? bholley: I know you're busy with stylo work, but could you point us in the right direction here? glandium updated our Mac builds to clang 3.9 and that seems to be the source of this regression.

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #7) > These tests seem to just add a bunch of CSS rules and call getComputedStyle, > so the regression must be either in DOM or style code: > https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/tests/ > perf-reftest/bloom-basic.html > https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/tests/ > perf-reftest/bloom-basic-ref.html > > I'm not really sure what it means that both tests regressed by about the > same amount, That's the intention (and why it's a "perf" reftest) - the idea is that the bloom filter is working if these two tests take roughly the same amount of time. > I guess that the bloom filter (which is the point of the test) > is still working as designed, but something else got slower? Precisely. > bholley: I know you're busy with stylo work, but could you point us in the > right direction here? glandium updated our Mac builds to clang 3.9 and that > seems to be the source of this regression. That's not entirely surprising. The performance of this stuff is highly-dependent on L2 cache locality, especially with Gecko's design. So I can totally imagine that an LLVM update might rearrange things to make them worse somehow. Stylo does several times better than Gecko on this testcase, so assuming the regression is only limited to this one test, it's a temporary problem, and I don't think we should gate a compiler upgrade on a microbenchmark. That said, if other stuff got slower as well, this might be a good place to start the investigation.

Specifically, stylo's cache-friendly design makes it 6x faster on this testcase without parallelism and style sharing.

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #9) > Specifically, stylo's cache-friendly design makes it 6x faster on this > testcase without parallelism and style sharing. (whoops, sorry - the 6x was with parallelism. It's a bit under 2x without the parallelism).

:jmaher Try backout results have arrived. Try baseline results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a47de40e7bc39b9f626813258424aad69dde021 Try backout results for 7e67993e7c5b changeset: https://treeherder.mozilla.org/#/jobs?repo=try&revision=03dab66ad13fef4882c37136b749c10224fb70f0 This compare view shows an aproximate 25% improvement after backout of 7e67993e7c5b: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=9a47de40e7bc&newProject=try&newRevision=03dab66ad13f&framework=1&showOnlyImportant=0 This shows changeset 7e67993e7c5b is regressive.

:ted as :glandium is on holiday for the next ~2 weeks, is this something you could look into?

To be clear, if this is the only regression from the compiler upgrade, I think we should swallow it and move on. Given that fixing the compiler isn't really an option, the correct solution to this would be to improve Gecko's selector representation to be more cache-friendly. But given how close we are on Stylo, I don't think that would be time or risk well-spent.

Given comment 13, can we close this?

sounds good to me.