Closed Bug 1139033 Opened 6 years ago Closed 6 years ago

3.58% regression on SVG-ASAP on OS X

Categories

(Core :: Graphics: Layers, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

(Keywords: perf, regression, Whiteboard: [talos_regression])

Attachments

(1 file)

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.
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.
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#
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.
Attached patch PatchSplinter Review
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
Attachment #8572858 - Flags: review?(nical.bugzilla)
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
Attachment #8572858 - Flags: review?(nical.bugzilla) → review+
https://hg.mozilla.org/mozilla-central/rev/89120ad0518f
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Duplicate of this bug: 1140473
Keywords: perf, regression
Whiteboard: [talos_regression]
You need to log in before you can comment on or make changes to this bug.