27.09 - 27.85% bloom_basic http: / bloom_basic_ref http: (osx-10-10) regression on push 03f394bbe821227153b23697825099022b601e8b (Wed Apr 19 2017)

RESOLVED WONTFIX

Status

defect
RESOLVED WONTFIX
2 years ago
Last year

People

(Reporter: igoldan, Unassigned)

Tracking

({perf, regression, talos-regression})

Firefox Tracking Flags

(Not tracked)

Details

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
Component: Untriaged → Build Config
Product: Firefox → Core
Does this perhaps match the regressions seen on cross-mac builds?
Flags: needinfo?(ted)
If this ever goes to the backout route, please only backout 7e67993e7c5b, not all three changesets from the push.
Assignee: mh+mozilla → nobody
(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.
Flags: needinfo?(ted)
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.
Flags: needinfo?(ted)
Flags: needinfo?(ionut.goldan)
:jmaher Sure thing.
Flags: needinfo?(ionut.goldan)
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.
Flags: needinfo?(ted) → needinfo?(bobbyholley)
(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.
Flags: needinfo?(bobbyholley)
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).
:ted as :glandium is on holiday for the next ~2 weeks, is this something you could look into?
Flags: needinfo?(kaku) → needinfo?(ted)
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?
Flags: needinfo?(jmaher)
sounds good to me.
Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(jmaher)
Resolution: --- → WONTFIX
Flags: needinfo?(ted)

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.