1139033 - 3.58% regression on SVG-ASAP on OS X

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

I got a regression email from the first landing of bug 1137203 on b-i:

Regression: B2g-Inbound - SVG-ASAP - MacOSX 10.6 (rev4) - 3.58% increase
------------------------------------------------------------------------
    Previous: avg 451.087 stddev 0.816 of 12 runs up to revision b36ee59d349e
    New     : avg 467.230 stddev 4.104 of 12 runs since revision ddf965a90c07
    Change  : +16.142 (3.58% / z=19.786)
    Graph   : http://mzl.la/1FPBNPP

It's a small regression but probably easily fixable so I'm filing this to track it. Note that bug 1137203 was backed out and then relanded so even though this regression went away it will come back.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

My try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=363d6de6c138 seems to indicate that the regression is from the second of the two patches. This is good because the first one shouldn't have any impact unless APZ is enabled, and therefore should have no impact on OS X. Most likely the removal of fast path is causing a repeat transaction in some cases where it wasn't before and that's slowing things down a tad.

Comment 2

[Vaibhav (:vaibhav1994)]

This shows a regression on alert manager (a tool for handling regression for talos): http://alertmanager.allizom.org:8080/alerts.html?rev=b94bcbc389e8&showAll=1&testIndex=0&platIndex=0#

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attempt at a fix, not sure if it'll do the job: https://treeherder.mozilla.org/#/jobs?repo=try&revision=95938e1a1daf

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Ugh. Trychooser is so annoying sometimes.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1858e920b3e1

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Ok so the patch seems to have worked. I'll do a full run to make sure it doesn't break anything else and post the patch for review.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

In bug 1137203 we landed a patch to get rid of the explicit "fast path" in the tiling code. Instead, we would just go through the high-precision painting path, and paint non-progressively, which was pretty much equivalent. All the things that used to make us go into the fast path now force us to paint non-progressively.

The difference though is that the fast path would just paint and return, whereas the longer path we go through now also schedules a repeat transaction if the paint isn't flagged as finished [1]. The progressive-painting path does flag it as finished after all the invalid areas has been painted, but the non-progressive painting path does not. That's what my patch does.

Without my patch, and with low-precision drawing disabled (which is the case on OS X), we schedule this extra repeat transaction which enters, discovers there's nothing to be painted, and immediately terminates. It's basically a no-op which is wasteful, and my patch avoids that. It should have no other impact.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/ClientTiledPaintedLayer.cpp?rev=da60b4e6c98b#494

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Fuller try run based on a recent m-c to make sure it doesn't break anything:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a5fe5a4a223a

Comment 8

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/89120ad0518f

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/89120ad0518f