Closed Bug 814272 Opened 12 years ago Closed 12 years ago

15-20fps regression in settings scrolling between 11-18 and 11-19 nightlies

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect, P3)

Product:
Firefox OS Graveyard
Component:
Gaia::Settings
Platform:
ARM
Gonk (Firefox OS)
Type:
defect

Tracking

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed, firefox20 fixed)

Status:
RESOLVED FIXED
Project Flags:
blocking-basecamp +
Tracking Flags:
Tracking Status
firefox18 --- fixed
firefox19 --- fixed
firefox20 --- fixed

People

(Reporter: cjones, Assigned: mattwoodrow)

References

Details

Attachments

(1 file, 1 obsolete file)

Cache the bounds of nsDisplayBackgroundImage v2
2.71 KB, patch
roc
: review+
Details | Diff | Splinter Review 
On my unagi with the 11-15 build from the stable channel, scrolling is a solid 60fps.  On my local build made from the equivalent of the 11-20 codebase, it's 40-45fps.

Had hoped this was bug 803910, but apparently not.

Paint flashing shows us invalidating the main area reasonably.  Not sure what changed.  We should start by bisecting.

gal, want to take this?

bb+ because 60fps scrolling here is one of the v1 product reqs.
Performance regressed between the 11-18 and 11-19 nightlies.
Summary: 15-20fps regression in settings scrolling between 11-15 and 11-20 → 15-20fps regression in settings scrolling between 11-18 and 11-19 nightlies
Suspicious things
 - bug 798964
 - bug 810470
 - bug 795648

Nothing looks suspicious in the gaia range.
Bisect sez

https://github.com/mozilla/releases-mozilla-aurora/commit/3b59d58b467f7166434887fdbfbbdeccfbd04734

Author: Matt Woodrow <mwoodrow@mozilla.com>
Date:   Thu Oct 25 18:32:25 2012 +1300

    Bug 798964 - Add nsDisplayBackground color and make the bounds of nsDisplayBackground match the image. r=roc,a=akeybl
Blocks: 798964
Would it be possible to grab the display list dumps (gfxUtils::sDumpPaintList) for scrolling before/after the regression?

Hopefully it's fairly obvious from that.
Bug 811173 might be related too.
(In reply to Matt Woodrow (:mattwoodrow) from comment #6)
> Bug 811173 might be related too.

No dice :(.
<mattwoodrow> cjones: it looks like the content is in an RGBA layer
<mattwoodrow> because the background doesn't scroll with it
<mattwoodrow> I *think* we have code that detects if the background is a solid color and copies it up, and it seems plausible that my change broke that
<mattwoodrow> I just need to remember where that code is, and debug it
<mattwoodrow> alternatively, give the scrolled frame a solid color background
BasicLayerManager (0x473bb190)
  BasicContainerLayer (0x4751b000) [visible=< (x=0, y=0, w=320, h=480); >] [opaqueContent] [metrics={ viewport=(x=0.000000, y=0.000000, w=320.000000, h=480.000000) viewportScroll=(x=0.000000, y=0.000000) displayport=(x=0.000000, y=0.000000, w=0.000000, h=0.000000) scrollId=0 }]
    BasicThebesLayer (0x4751b400) [clip=(x=0, y=0, w=0, h=0)] [not visible] [isFixedPosition]
    BasicColorLayer (0x4260e640) [clip=(x=0, y=0, w=0, h=0)] [not visible] [opaqueContent] [isFixedPosition] [color=rgba(0, 0, 0, 1)]
    BasicContainerLayer (0x48452800) [clip=(x=0, y=0, w=320, h=480)] [visible=< (x=0, y=0, w=320, h=480); >] [opaqueContent] [isFixedPosition]
      BasicThebesLayer (0x48453000) [visible=< (x=0, y=0, w=320, h=480); >] [opaqueContent] [isFixedPosition] [valid=< (x=0, y=0, w=320, h=480); >]
Andreas: Is that from before the regression? And have you recently scrolled the settings app, it looks like the scrolling has gone inactive from that layer tree.
Assignee: nobody → kaze
Unassigning myself as I have no chance to work on this before 2 weeks at least.
Assignee: kaze → nobody
Assignee: nobody → matt.woodrow
comment 10: this is after the change. I will make sure to capture again during scrolling. Can you explain why rgba is slower than rgb?
I had a look at these locally, there's no obvious changes to the layer tree, nor does paint flashing show anything interesting.

I believe we're using 565 (16bit) for rgb on b2g, so it's significant reduction in memory usage/bandwidth over rgba (32bit).

I profiled scrolling: http://people.mozilla.com/~bgirard/cleopatra/#report=16b843f15d1bee536c8ac0b233237a08accc7912

Looks like we're spending a significant amount of time in ProcessDisplayItems which is unexpected, it shouldn't be overly expensive. I can't see any obvious reasons that my patch would have made that worse though.
Can you grab a profile from the cset before the regression?  Then we can compare the profiles to see where we're spending more time.
(Goes without saying, but keeping the gaia commit constant, of course.)
(In reply to Matt Woodrow (:mattwoodrow) from comment #13)
> I profiled scrolling:
> http://people.mozilla.com/~bgirard/cleopatra/
> #report=16b843f15d1bee536c8ac0b233237a08accc7912
> 

The libxul symbolication in that profile looks pretty nuts, fyi.
Andreas is going to grab differential profiles to start with.
Assignee: matt.woodrow → gal
Definitely looks like ProcessDisplayItems has moved up a lot.
Assignee: gal → matt.woodrow
Profile with more sample labels:

http://people.mozilla.com/~bgirard/cleopatra/#report=bed155c6e2d773095270277d593e351760ac7f54&filter=%255B%257B%2522type%2522%253A%2522RangeSampleFilter%2522%252C%2522start%2522%253A1748%252C%2522end%2522%253A2590%257D%255D

Looks like around ~15% of the profile is within nsDisplayBackgroundImage::GetBounds().
Looks like this fixes the regression.
Attachment #685422 - Flags: review?(roc)
\o/
Cache the bounds in the constructor instead of doing it lazily.
Attachment #685422 - Attachment is obsolete: true
Attachment #685422 - Flags: review?(roc)
Attachment #685428 - Flags: review?(roc)
https://hg.mozilla.org/integration/mozilla-inbound/rev/3edb5bb92461
https://hg.mozilla.org/mozilla-central/rev/3edb5bb92461
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
I was wondering why scrolling was still crap in my latest build and now I see why :).  This bug seems to be hiding from the uplift queries.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: