629475 - Regression: displayport takes longer to refresh

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Benjamin Stover (:stechz)]

I am testing a patch on bug 629146, and since yesterday platform has regressed with refreshing content.

It was working great here: http://hg.mozilla.org/mozilla-central/rev/5a0f46c26d12
But not here: http://hg.mozilla.org/mozilla-central/rev/09bfc85b151c

I'm triaging to find the bad revision.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This bug report is short on information.  Are you observing the slowdown while panning?  Or when, say, two-finger swiping to top-bottom to force a full-window repaint?

Comment 2

[Benjamin Stover (:stechz)]

The slowdown is while panning, and it's an obvious regression on my phone when panning around on most sites. For instance, with the moving-target patch and yesterday's build, content would refresh less than a second after beginning the pan. Today, the pan will pretty much finish before it shows any content. The only thing that changed was platform.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

OK.  The regression range includes drawing the checkerboard.  It might be that the browser process is chewing more CPU now and choking out the content process.  Could test by commenting early-returning from BuildBackgroundPatternFor() in RenderFrameParent.

Comment 4

[Benjamin Stover (:stechz)]

OK, confirmed that it's the checkerboard.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

As we discussed on IRC, a simple tuning knob here is to make the background image larger.  Ideally cairo would be smart about this, but spitting in one hand etc.

There's a great perf-measurement class in js/src/perf/jsperf.h that ought to work in >= 2.2, which would allow tuning the size quantitatively.  Start it counting before the background fill, stop after, record cycle counts + cache missed, lather rinse repeat.

I'd be fine with disabling checkerboard again in the meantime.

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(And of course, sadly, GL would make this moot.)

Comment 7

[Pedro Alves]

I can confirm the regression from b3 to b4 on android. 

I would also like to point out that from a perception point of view, the
checkerboard background "feels slower" than a white background

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

If we're set on keeping the checkerboard, then comment 5 is probably easiest way to cut down on CPU.  It can be measured quantitatively too.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch - 64px checkerboard

Did I do this right? :)

I wanted to bump the image from 16px to 64px and see how the paints worked. On planet.mozilla.org it looks like it is painted faster and I see less small block paints.

Comment 10

[Pedro Alves]

Is there / will be any option in about:config to turn off the checkerboard completely?

Comment 11

[Matt Brubeck (:mbrubeck)]

Bug 627828 is about making the background (checkerboard) image customizable.

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 511843 [details] [diff] [review]
patch - 64px checkerboard

This patch could be made smaller with more macro-ization, but it's a hack on a hack so I'm inclined not to care.

It's not very hard to measure the perf difference quantitatively, even without jsperf, e.g. with a TimeStamp before and after drawing the checkerboard.

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

I was able to add some timing code here:
http://mxr.mozilla.org/mozilla-central/source/gfx/layers/basic/BasicLayers.cpp#730

Normal painting was fast. But painting while panning seemed to be slower. This is in both cases - with and without patch.

Without the patch, painting while panning was ~25-30 ms
With patch, painting while panning was ~12-18 ms

So we could be averaging ~44% better with the patch. Good enough to land.

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mozilla-central/rev/525db94bf558

let's file new bugs on specific issues

Comment 15

[Andreea Pod]

Verified on build: Mozilla /5.0 (Android;Linux armv7l;rv:2.2a1pre)
Gecko/20110407 Firefox/4.2a1pre Fennec /4.1a1pre 

Device: Samsung Captivate (Android 2.2 update 1)