Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#9617 closed enhancement (fixed)

[patch][ccla]_docScroll performance improvement

Reported by: Douglas Hays Owned by: Douglas Hays
Priority: high Milestone: 1.4
Component: Core Version: 1.3.2
Keywords: Cc:
Blocked By: Blocking:

Description

The current _docScroll has some builtin performance penalties.

var _b = d.body(), _w = d.global, de = d.doc.documentElement;
return {y: (_w.pageYOffset || de.scrollTop || _b.scrollTop || 0),
        x: (_w.pageXOffset || d._fixIeBiDiScrollLeft(de.scrollLeft) || _b.scrollLeft || 0)};

Penalties:
1) body() and doc.documentElement are computed all the time, even when not needed.
2) if the values for x and y are 0, then all components of the assignment are evaluated.
3) if pageYOffset is not used, then pageXOffset is still evaluated (similarly for documentElement.scrollLeft/Top).
I suggest using the following code instead:

var _n = d.global;
return "pageXOffset" in _n? { x:_n.pageXOffset, y:_n.pageYOffset } :
        (_n=d.doc.documentElement, _n.clientHeight? { x:d._fixIeBiDiScrollLeft(_n.scrollLeft), y:_n.scrollTop } :
        (_n=d.body(), { x:_n.scrollLeft||0, y:_n.scrollTop||0 }));

All penalties are removed but there are about 40 more bytes added to html.js.
Results:

IE6: 20% performance improvement (all cases),
FF2: 20% improvement when scrollLeft/Top != 0, 50% improvement when scrollLeft/Top = 0,
Safari4: 50% improvement when scrollLeft/Top != 0, 80% improvement when scrollLeft/Top = 0,
Opera9.6: 25% improvement when scrollLeft/Top != 0, 75% improvement when scrollLeft/Top = 0,

Change History (6)

comment:1 Changed 10 years ago by James Burke

I'm fine with the change (maybe call _n just n to save a couple of bytes, but probably ends up being a wash when gzippped), as long as whatever tests you used to track the performance improvement were also checked in as DOH tests (if they are not already) to confirm the right return values for the various cases.

comment:2 Changed 10 years ago by Douglas Hays

Owner: changed from anonymous to Douglas Hays
Status: newassigned

comment:3 Changed 10 years ago by Douglas Hays

Resolution: fixed
Status: assignedclosed

(In [19114]) Fixes #9617 !strict. Optimize _docScroll to reduce needless evaluations. Add html_docScroll.html automated test.

comment:4 Changed 10 years ago by Douglas Hays

Milestone: tbd1.4

comment:5 Changed 8 years ago by bill

In [26144]:

disable testSpeed() test until it passes on IE8, refs #9617, #11200.

comment:6 Changed 8 years ago by bill

In [26154]:

re-enable testSpeed() test, the IE8 failure is a regression, refs #9617, #11200.

Note: See TracTickets for help on using tickets.