292431 - 15% performance regression on DHTML due to fix for bug 240276

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[José Jeria]

Build id: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2)
Gecko/20050429 Firefox/1.0+

This regressed due to bug 240276, see bug 240276, comment 32.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Here's the Tdhtml data from just before the checkin.

----------- Output from DHTMLPerformanceTest ------------- 
  Document http://www.mozilla.org/performance/test-cases/dhtml/runTests.html
loaded successfully
  Test           Median Average Data
  ============================================================
  colorfade:     1738   1760    1859,1738,1736,1740,1729
  diagball:      1909   1907    1890,1907,1909,1912,1915
  fadespacing:   2859   2851    2819,2863,2859,2855,2860
  imageslide:    414    414     405,408,425,414,419
  layers1:       5382   5397    5399,5377,5369,5459,5382
  layers2:       230    230     227,232,232,227,230
  layers4:       227    228     231,227,229,227,227
  layers5:       4774   4807    4862,4770,4774,4866,4762
  layers6:       576    576     574,576,572,578,578
  meter:         1760   1818    2063,1760,1749,1760,1757
  movingtext:    1340   1335    1312,1341,1339,1340,1343
  mozilla:       3176   3213    3096,3439,3176,3183,3173
  replaceimages: 3964   4023    3997,3938,4252,3962,3964
  scrolling:     13916  14307   13832,13875,13916,15703,14210
  slidein:       2860   2861    2859,2864,2864,2858,2860
  slidingballs:  3978   3979    3982,3971,3978,3991,3974
  zoom:          688    690     699,687,688,690,686
  _x_x_mozilla_dhtml,1717
----------- End Output from DHTMLPerformanceTest --------- 

and just after

----------- Output from DHTMLPerformanceTest ------------- 
  Document http://www.mozilla.org/performance/test-cases/dhtml/runTests.html
loaded successfully
  Test           Median Average Data
  ============================================================
  colorfade:     2272   2298    2433,2263,2248,2275,2272
  diagball:      2745   2747    2747,2738,2786,2720,2745
  fadespacing:   3956   3970    3931,4028,3935,4001,3956
  imageslide:    605    608     573,642,636,605,586
  layers1:       5688   5713    5752,5669,5671,5787,5688
  layers2:       231    231     229,232,232,230,231
  layers4:       230    231     233,230,229,231,230
  layers5:       4724   4820    4807,4724,4715,5130,4723
  layers6:       577    578     576,576,577,579,581
  meter:         2858   2900    2879,2817,3124,2858,2820
  movingtext:    1979   1981    1996,1977,1975,1979,1980
  mozilla:       3967   4236    3924,5363,3968,3958,3967
  replaceimages: 3865   3939    4230,3863,3861,3865,3877
  scrolling:     13991  13984   13909,13962,13991,14023,14036
  slidein:       3886   3881    3870,3858,3899,3886,3890
  slidingballs:  4076   4066    4020,4076,4079,4076,4077
  zoom:          992    992     995,991,992,991,992
  _x_x_mozilla_dhtml,2070
----------- End Output from DHTMLPerformanceTest --------- 

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

The cause of the performance regression was that we are resizing the scrolled
frame's view after reflowing it (which gives it the size of the scrolled
frame's overflow area). Then later we resize it to the size of the scrollport
which is the correct final size. When the scrolled frame is shorter than the
viewport, this resizing forces us to repaint the area of the viewport below the
scrolled frame's bottom edge every time we reflow anything. When you combine
this with existing dirty rectangles in the viewport, parts of the scrolled
frame can be repainted unnecessarily as well.

The solution is to not resize the scrolled frame's view after reflowing it. It
will be sized and positioned by PlaceScrollArea.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 182747 [details] [diff] [review]
fix

need approval for this serious perf. regression

Comment 4

[chris hofmann]

Comment on attachment 182747 [details] [diff] [review]
fix

a=chofmann

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

luna went back to 1691 so all is well.