1329313 - 3.03 - 3.13% tsvgr_opacity (linux64) regression on push 1e8f1baf3894 (Wed Jan 4 2017)

Status: RESOLVED WONTFIX

(Core :: Widget: Gtk, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

Talos has detected a Firefox performance regression from push 1e8f1baf389407438d6a57a62228e32a37c5623f. As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

  3%  tsvgr_opacity summary linux64 pgo          369.5 -> 381.07
  3%  tsvgr_opacity summary linux64 pgo e10s     336.46 -> 346.67


You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=4729

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

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

:karlt, I see you are the patch author of bug 1328352.  This seems to be PGO only (not opt) and after backfilling for pgo and data, it showed this patch as the root cause.

Can you take a look at this and see why just this one test is problematic?  It seems to be in the subtest:
tsvgr_opacity small-group-opacity-2500.svg

Comment 2

[Karl Tomlinson (:karlt)]

In
https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bautoland,5672b657c5b23ed1c359257e1722e2ed25c7e785,1,1%5D&zoom=1483284374322.7383,1483441464833.7407,363.9130434782609,390&selected=%5Bautoland,5672b657c5b23ed1c359257e1722e2ed25c7e785,156471,65798875,1%5D
this regression looks like the reversal of improvements reflected in
https://treeherder.mozilla.org/perf.html#/alerts?id=4700 presumably from
changes for bug 1320860.

As bug 1328352 fixed an accidental regression caused by changes for bug
1320860, the reversal of the performance improvement may be kind of expected.

Still I didn't expect Ubuntu 12.04 with default GTK theme Ambiance to be
affected by the regression in bug 1328352, so the changes in performance are
not really expected.

I pushed some logging to try to see what change in values was caused by https://hg.mozilla.org/mozilla-central/rev/1bf65dbb4d71

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9e04976dfee0d76bcacd5bd297f4645c0fc75f1d
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab3ce5a847ece6d5319c504d9506514a543a9679

If that shows no change in values, as expected, then I think this performance change should just be written off as a PGO quirk.  If changes for bug 1328352 are backed out, then those for bug 1320860 will also need to be, in which case we'll still have the same performance anyway.

Comment 3

[Karl Tomlinson (:karlt)]

(In reply to Joel Maher ( :jmaher) from comment #1)
> :karlt, I see you are the patch author of bug 1328352.  This seems to be PGO
> only (not opt) and after backfilling for pgo and data, it showed this patch
> as the root cause.

Comparing 1bf65dbb4d71 with its ancestor 3b5eab628d4b agrees that there was a
significant 2.5% change in linux64 pgo tsvgr_opacity with and without e10s:

tsvgr_opacity pgo
 e10s?   Base             New             Delta  Confidence   # Runs  
  no     371.51 ± 0.57% < 381.09 ± 0.35%  2.58%  13.46 (high) 16 / 8  
 yes     338.78 ± 0.95% < 346.99 ± 0.56%  2.42%   6.16 (high)  8 / 8

https://treeherder.mozilla.org/perf.html#/compare?originalProject=autoland&originalRevision=3b5eab628d4b&newProject=autoland&newRevision=1bf65dbb4d71ccc02ad63b95a039112073d83e5e&framework=1&filter=svg%20linux&showOnlyImportant=0

> Can you take a look at this and see why just this one test is problematic? 

The change is in the result for subtest big-optimizable-group-opacity-2500.svg

https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=autoland&originalRevision=3b5eab628d4b&newProject=autoland&newRevision=1bf65dbb4d71ccc02ad63b95a039112073d83e5e&originalSignature=5672b657c5b23ed1c359257e1722e2ed25c7e785&newSignature=5672b657c5b23ed1c359257e1722e2ed25c7e785&framework=1
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=autoland&originalRevision=3b5eab628d4b&newProject=autoland&newRevision=1bf65dbb4d71ccc02ad63b95a039112073d83e5e&originalSignature=bbf8e5d8267164f7ecd222bc41cf2b93e9df7303&newSignature=bbf8e5d8267164f7ecd222bc41cf2b93e9df7303&framework=1

I've looked at big-optimizable-group-opacity-2500.svg and there is nothing in
that file that I would expect to be affected by 1bf65dbb4d71.

Comment 4

[Karl Tomlinson (:karlt)]

An fprintf added in
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9e04976dfee0
confirms that, on the Ubuntu 12.04 test machines, the line of code added in
1bf65dbb4d71 merely writes the value 1.0 to a local variable that already has
the value 1.0.

Comparing results from the revision with that fprintf against its ancestor
cfe1c5c0caeb32c50552d9886a9afda6300819fc shows that the added fprintf
reinstates the performance improvement.

tsvgr_opacity pgo
 e10s?   Base             New             Delta  Confidence   # Runs  
  no     380.14 ± 0.08% > 370.44 ± 0.23% -2.55%  20.99 (high)  4 / 4  
 yes     343.40 ± 0.78% > 337.65 ± 1.29% -1.67%   2.32 (low)   5 / 4

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=cfe1c5c0caeb32c50552d9886a9afda6300819fc&newProject=try&newRevision=9e04976dfee0&framework=1&filter=svg&showOnlyImportant=0

Backing out 1bf65dbb4d71 from 9e04976dfee0 with fprintf is ab3ce5a847ec, which
is not significantly different from 9e04976dfee0.

tsvgr_opacity pgo
 e10s?   Base             New             Delta  Confidence   # Runs  
  no     370.44 ± 0.23% < 371.39 ± 0.55%  0.25%   0.85 (low)  4 / 4
 yes     337.65 ± 1.29% > 335.24 ± 0.97% -0.71%   0.89 (low)  4 / 4

Changing ApplyColorOver() in nsLookAndFeel.cpp, even if just adding fprintf, is somehow affecting the performance of big-optimizable-group-opacity-2500.svg but, as indicated by the logging, ApplyColorOver() runs only once before the performance tests run and is returning the same results before and after 1bf65dbb4d71.

I can only speculate that somehow what is compiled in ApplyColorOver() is
affecting how other code is generated or running in
big-optimizable-group-opacity-2500.svg.  The fact that the performance change
is PGO-only is consistent with this hypothesis.

I guess an uninitialized variable may be a possible explanation, but
correctness tests are not detecting any failures, and I've run a valgrind
build on big-optimizable-group-opacity-2500.svg without detecting any
such problems.

Given that this seems to be a strange interaction with the compiler, the
changes are limited to affecting one test, and the regression merely returns
to performance levels before an unexpected improvement, I don't think it is
worth spending time analysing the compiler to recover an improvement, which
may then disappear on a subsequent random change.