25811 - performance problems dragging gfx scrollbars

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I did some profiling using jprof of dragging with gfx scrollbars.  That is, I
loaded my homepage, started profiling, dragged the slider from top to bottom to
top repeatedly, and quit.  I did this in a debug build, so the data could be a
little bit suspect.  The full data are at
http://dbaron.student.harvard.edu/u/david/mozperf/scrolling_gfx_withmyfix.html
(when my computer is on).

The basic problem seems to be that you're using an attribute to store the
position of the GFX scrollbar, and this causes a re-resolution of the style
context (since the matching attribute selectors could change).  In the above
profile, most of the time is divided between the following three functions,
which I think are mutually exclusive (note that you can use the # in the first
column as an ID selector at the end of the URL for the profile data):

189730   0       46 nsSliderFrame::AttributeChanged
177467   0      104 FrameManager::ComputeStyleChangeFor
178244   0       80 nsGfxScrollFrameInner::AttributeChanged

It seems to me (from browsing the profile data) that these three things are,
respectively:
 * repainting the scrollbar
 * re-resolving the style context
 * repainting the document
I think they're mutually exclusive since they are a division of the time spent
within
190332   2      232 nsDocument::AttributeChanged
which is slightly more than 80% of the time in the profile.

If that's true, then you're spending an awful lot of time doing something I
don't think you need to do.

Comment 1

[Peter Trudelle]

assigned to evaughan as p4 m18, but I doubt we'd hold any milestone for this. I 
will consider reevaluating this if it is perceived to be a performance problem 
in optimized builds.

Comment 2

[Chris Waterson]

dbaron: this is a good find. maybe we should whack a special-purpose API onto
the scrollbar to avoid using attributes to communicate the scrollbar position?
hyatt, whatdya think?

Comment 3

[David Hyatt]

Let's make these properties instead of attributes.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

For the record, profiling with an optimized build shows roughly the same
results:
http://dbaron.student.harvard.edu/~david/mozperf/scroll-homepage-drag.html#82465
http://dbaron.student.harvard.edu/~david/mozperf/scroll-homepage-drag.html#85882

The key bits are that the time (214 hits of 259 on main) in
nsXULElement::SetAttribute is split between:
 73666   1       91 nsGfxScrollFrameInner::AttributeChanged
 72911   1       78 FrameManager::ComputeStyleChangeFor
 85264   1       43 nsSliderFrame::AttributeChanged

This is showing the results of the smooth scrolling that is implemented on GFX
scrollbars - since very little is painted each step of the scrolling, the
performance of other things (the scrollbar itself) matters.

Comment 5

[Stuart Parmenter]

reassigning to me, M14 beta1.  scrolling is too slow to use on pages that
contain anything more than text.  don't even get me started on mailnews
scrolling on linux.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I would think fixing this problem would help by the same *amount* on all pages
(rather than proportion).  (I still think it's worth fixing, and probably not
too hard.)

Could you give an example of a page where scrolling is particularly slow? 
(Profile data could show something else interesting...)

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually, ignore my previous comment.  I think the scrollbars are part of the
document, not the chrome, so it would be the document's style context that's
relevant...

Comment 8

[Stuart Parmenter]

i'm doing lots of quantifying on all of this so i'm on top of the problems.

Comment 9

[Laura Yecies]

pdt would like more data please

Comment 10

[leger]

Putting on [NEED INFO] radar.

Comment 11

[Stuart Parmenter]

this doesn't need to be fixed for beta1.  if the bug this one depends on gets
fixed, this shouldn't matter enough for performance.  taking off the beta1
list.  let me know if you disagree with this and I'll put it back.

Comment 12

[Stuart Parmenter]

hyatt, you can fix this right? :)

Comment 13

[Paul MacQuiddy]

*IGNORE* - massive spam changing open XPToolkit bug's QA contact to
[e-mail]

Comment 14

[David Hyatt]

claudius observed that scrolling degrades dramatically on Mac between view 
manager 1 and 2.  This seems to be dominating the slowdown on Mac/Linux.  
Throwing to beard for analysis.

Comment 15

[Patrick C. Beard]

Reassigning to kevin.

Comment 16

[Stuart Parmenter]

er, kevin, not peter :)

Comment 17

[Kevin McCluskey (gone)]

Bulk moving to M16

Comment 18

[Kevin McCluskey (gone)]

I tested scrolling performance using view manager 1 and view manager 2 on WINNT, 
Mac, and Linux. I don't see any difference in performance. I do see a difference 
when native scrollbars are used instead of gfx scrollbars. The native scrollbars 
are faster.

Reassigning to evaughan.

Comment 19

[Kevin McCluskey (gone)]

*** Bug 32491 has been marked as a duplicate of this bug. ***

Comment 20

[Eric Vaughan]

Mike,

Does you fix to style fix this? Can you confirm it?

Comment 21

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Sure seems faster to me.  Mr Baron, are you satisfied?

Comment 22

[Peter Trudelle]

Moving all bugs that are not dogfood+, nsbeta2+,features, or nsbeta2- to M21

Comment 23

[Eric Vaughan]

fixed

Comment 24

[Matt Perry]

Verified duplicate.

Comment 25

[Matt Perry]

Oops.  Reopening as I marked wrong bug.  Sorry for the spam.

Comment 26

[Peter Trudelle]

resolving as fixed per evaughan's comment.

Comment 27

[John Morrison]

verified fixed.