performance problems dragging gfx scrollbars

VERIFIED FIXED

Status

()

P2
major
VERIFIED FIXED
19 years ago
18 years ago

People

(Reporter: dbaron, Assigned: eric)

Tracking

({perf, platform-parity})

Trunk
x86
Linux
perf, platform-parity
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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.
Keywords: perf

Comment 1

19 years ago
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.
Assignee: trudelle → evaughan
Priority: P3 → P4
Target Milestone: M18

Comment 2

19 years ago
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

19 years ago
Let's make these properties instead of attributes.
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

19 years ago
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.
Assignee: evaughan → pavlov
Severity: normal → critical
Keywords: beta1, pp
Priority: P4 → P1
Target Milestone: M18 → M14
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...)
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

19 years ago
i'm doing lots of quantifying on all of this so i'm on top of the problems.
Status: NEW → ASSIGNED

Updated

19 years ago
Depends on: 26502

Comment 9

19 years ago
pdt would like more data please

Comment 10

19 years ago
Putting on [NEED INFO] radar.
Whiteboard: [NEED INFO]

Comment 11

19 years ago
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.
Severity: critical → major
Keywords: beta1
Priority: P1 → P2
Whiteboard: [NEED INFO]

Comment 12

19 years ago
hyatt, you can fix this right? :)
Assignee: pavlov → hyatt
Status: ASSIGNED → NEW

Comment 13

19 years ago
*IGNORE* - massive spam changing open XPToolkit bug's QA contact to
jrgm@netscape.com
QA Contact: paulmac → jrgm

Updated

19 years ago
Status: NEW → ASSIGNED
Target Milestone: M14 → M15

Comment 14

19 years ago
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.
Assignee: hyatt → beard
Status: ASSIGNED → NEW

Comment 15

19 years ago
Reassigning to kevin.
Assignee: beard → trudelle

Comment 16

19 years ago
er, kevin, not peter :)
Assignee: trudelle → kmcclusk

Updated

19 years ago
Status: NEW → ASSIGNED
Bulk moving to M16

Updated

19 years ago
Target Milestone: M15 → M16
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.
Assignee: kmcclusk → evaughan
Status: ASSIGNED → NEW
*** Bug 32491 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 20

19 years ago
Mike,

Does you fix to style fix this? Can you confirm it?
Status: NEW → ASSIGNED
Sure seems faster to me.  Mr Baron, are you satisfied?

Comment 22

19 years ago
Moving all bugs that are not dogfood+, nsbeta2+,features, or nsbeta2- to M21
Target Milestone: M16 → M21
(Assignee)

Comment 23

19 years ago
fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Comment 24

18 years ago
Verified duplicate.
Status: RESOLVED → VERIFIED

Comment 25

18 years ago
Oops.  Reopening as I marked wrong bug.  Sorry for the spam.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---

Comment 26

18 years ago
resolving as fixed per evaughan's comment.
Status: REOPENED → RESOLVED
Last Resolved: 19 years ago18 years ago
Resolution: --- → FIXED

Comment 27

18 years ago
verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.