Alison Shiue Reporter
	Description • 8 years ago

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

Summary of tests that regressed:

  tp5o_scroll summary linux64 opt e10s: 5.02 -> 5.2 (3.67% worse)
  tscrollx summary linux64 opt e10s: 3.4 -> 3.77 (10.95% worse)
  cart summary linux64 opt e10s: 30.78 -> 32.15 (4.44% worse)
  cart summary linux64 opt: 28.66 -> 29.99 (4.63% worse)
  tsvgr_opacity summary linux64 pgo: 381.8 -> 414.35 (8.53% worse)
  cart summary linux64 pgo: 21.27 -> 22.72 (6.81% worse)
  tsvgr_opacity summary linux64 pgo e10s: 343.78 -> 374.69 (8.99% worse)
  tscrollx summary linux64 pgo e10s: 3.39 -> 3.54 (4.2% worse)
  cart summary linux64 pgo e10s: 22.91 -> 24.45 (6.73% worse)
  tp5o_scroll summary linux64 pgo e10s: 4.87 -> 4.99 (2.63% worse)
  tart summary linux64 opt: 5.44 -> 5.57 (2.43% worse)
  damp summary linux64 opt e10s: 257.83 -> 263.87 (2.34% worse)
  tresize linux64 pgo: 20.61 -> 23.3 (13.03% worse)
  tresize linux64 opt: 21.69 -> 25.27 (16.52% worse)

Summary of tests that improved:

  tps summary linux64 opt e10s: 51.14 -> 45.84 (10.37% better)
  tp5o_scroll summary linux64 opt: 8.59 -> 7.65 (10.93% better)
  tps summary linux64 opt: 68.49 -> 63.69 (7.01% better)
  tsvgx summary linux64 opt e10s: 227.13 -> 192.83 (15.1% better)
  tsvgr_opacity summary linux64 opt e10s: 487.17 -> 434.14 (10.88% better)
  tsvgx summary linux64 opt: 358.25 -> 285.42 (20.33% better)
  tsvgr_opacity summary linux64 opt: 551.16 -> 495.01 (10.19% better)
  tscrollx summary linux64 opt: 7.44 -> 6.7 (10% better)
  tcanvasmark summary linux64 pgo e10s: 10468.08 -> 11370.62 (8.62% better)
  tcanvasmark summary linux64 pgo: 10177.58 -> 11023.88 (8.32% better)
  tsvgx summary linux64 pgo: 305.18 -> 232.95 (23.67% better)
  tsvgx summary linux64 pgo e10s: 188.22 -> 152.58 (18.94% better)
  tps summary linux64 pgo: 57.22 -> 52.14 (8.87% better)
  tps summary linux64 pgo e10s: 44.65 -> 38.79 (13.13% better)
  tp5o_scroll summary linux64 pgo: 7.41 -> 7.24 (2.21% better)


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

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

	Alison Shiue Reporter
	Comment 1 • 8 years ago

After did some retriggers, this issue might be caused by push:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f9243f28dbe963e88bd2631a631f3344bcb29e5e&tochange=33b8617a72339f68b7b77f10e972f94d92e57609

Hi Lee, as you are the patch author, can you take a look at this and determine what is the root cause? Thanks!

	Lee Salzman [:lsalzman] Assignee
	Comment 2 • 8 years ago

Note that the Skia content patch got backed out already, and thus there is no immediate concern.. I am still investigating these regressions, but will be on PTO till the middle of next week. I will look into this then and update with more information.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 3 • 8 years ago

ah, I see an alert for the backout:
https://treeherder.mozilla.org/perf.html#/alerts?id=2400

for this bug, should we close it since it is not on the tree, or do you want to leave it open as reference for when the skia patch lands again?

	Lee Salzman [:lsalzman] Assignee
	Comment 4 • 8 years ago

(In reply to Joel Maher ( :jmaher) from comment #3)
> ah, I see an alert for the backout:
> https://treeherder.mozilla.org/perf.html#/alerts?id=2400
> 
> for this bug, should we close it since it is not on the tree, or do you want
> to leave it open as reference for when the skia patch lands again?

Let's leave this open as I'll try to investigate these performance issues before I do another push.

	Lee Salzman [:lsalzman] Assignee
	Comment 5 • 8 years ago

The default shader blitter, which gets used when we call SkCanvas::drawRect, can cause use to temporarily buffer the bitmap shader output, then separately blend that out. This is more expensive than the sprite blitter in Skia (which writes directly to the output).

The compositor especially relies upon FillRect and does a lot of 1:1 blits that can be redirected to the sprite blitter, so this helps a lot there.

	Lee Salzman [:lsalzman] Assignee
	Comment 6 • 8 years ago

Clamp_S32_opaque_D32_nofilter_DX_shaderproc is slow, and we don't want to use it vs. the default clamp/nofilter shader proc. The default never got used because of this ordering bug, which costs us some performance especially in the compositor.

	Lee Salzman [:lsalzman] Assignee
	Comment 7 • 8 years ago

So between these two fixes related to compositor blits, and the unbounded operator handling issue fixed in bug 1296301, this is about all we can do related to mitigating some of these regressions. Otherwise, we believe that whatever regressions remain are tolerable and outweighed by the other benefits of moving to Skia  for content rendering.

	Pulsebot
	Comment 8 • 8 years ago

Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b166347d986
add DrawSurface fast path for DrawTargetSkia::FillRect. r=mchang
https://hg.mozilla.org/integration/mozilla-inbound/rev/c792d3cb0322
prefer default shader procs over special-cases in SkBitmapProcState. r=mchang

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 9 • 8 years ago

I see some improvements coming in already:

== Change summary for alert #2674 (as of August 23 2016 10:55 UTC) ==

Summary of tests that improved:

  tps summary linux64 opt: 81.18 -> 73.72 (9.18% better)
  tsvgx summary linux64 opt: 376.36 -> 292.75 (22.22% better)
  tsvgr_opacity summary linux64 opt: 539.96 -> 486.53 (9.9% better)

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=2674


we will see if others come in, this is pretty early on- I can try to verify everything once it is merged to central and this bug is marked as resolved.

	Ryan VanderMeulen [:RyanVM]
	Comment 10 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/5b166347d986
https://hg.mozilla.org/mozilla-central/rev/c792d3cb0322