Patch: Scroller slow IE7 scrolling performance

jlabanca

Scrolling between "pages" is unusably slow in IE7 if the table has too many columns (yes, I mean columns, now rows). A lot of the performance problem is caused by a snippet of code where the dataTable measures the widths of the hidden nSizers on the data table and applies the width to the real headers. The width of each header is read and modified in the same loop, which forces the browser to calculate layout for the entire table on every pass! For a table with 10 columns, that means 10 layouts of a large table.

This new version combines all of the reads and writes into separate loops. It doesn't solve the performance completely (there is still some lag when scrolling), but at last makes the performance more linear. For me, it shaved off multiple seconds when scrolling between pages.


REPLACE THIS SNIPPET IN _fnScrollDraw() (comments are mine):
[code]
_fnApplyToChildren( function(nSizer, nToSize) {
// Setting the style invalidates layout for the
// entire table.
oStyle = nSizer.style;
oStyle.paddingTop = "0";
oStyle.paddingBottom = "0";
oStyle.borderTopWidth = "0";
oStyle.borderBottomWidth = "0";
oStyle.height = 0;

// Reading the width of the header forces the
// browser to perform layout (very slow). Happens
// in every iteration because we just invalidated layout.
iWidth = $(nSizer).width();

nToSize.style.width = _fnStringToCss( iWidth );
aApplied.push( iWidth );
}, anHeadSizers, anHeadToSize );
[/code]

WITH THIS SNIPPET:
[code]
// Apply all styles in one pass. Invalidates layout
// only once because we don't read any DOM properties.
_fnApplyToChildren( function(nSizer, nToSize) {
oStyle = nSizer.style;
oStyle.paddingTop = "0";
oStyle.paddingBottom = "0";
oStyle.borderTopWidth = "0";
oStyle.borderBottomWidth = "0";
oStyle.height = 0;
}, anHeadSizers, anHeadToSize );

// Read all widths in next pass. Forces layout only
// once because we do not change any DOM properties.
_fnApplyToChildren( function(nSizer, nToSize) {
$(nSizer).data("iWidth", $(nSizer).width());
}, anHeadSizers, anHeadToSize );

// Apply all widths in final pass. Invalidates layout
// only once because we do not read any DOM properties.
_fnApplyToChildren( function(nSizer, nToSize) {
var iWidth = $(nSizer).data("iWidth");
$(nSizer).removeData("iWidth");
nToSize.style.width = _fnStringToCss( iWidth );
aApplied.push( iWidth );
}, anHeadSizers, anHeadToSize );
[/code]

allan

Absolutely superb - thank you very much for highlighting this and the fix! I've just committed it to DataTables ( https://github.com/DataTables/DataTables/commit/c2af41140b1a575e9e9193074cb95e76b72a4ee9 ) and it will be in 1.9.4 when released (not to far away). It is currently available in the 1.9.4.dev nightly on the downloads page.

To give an indication of the performance benefit, I've been working with a table that has a lot of columns and a fairly complex header / footer. A call to 'fnDraw' without this change took 3.5 seconds in Safari, while with the change it took just 1 second. Brilliant - thanks again!

Regards,
Allan

timtucker

Was looking through this and was thinking that maybe there's an alternate approach that would be even faster... tried so far and it seems to work, but would need to be tested a little more extensively.

What I did was add the following css classes:
[code]
.dataTables_hiddenSizer td, .dataTables_hiddenSizer th {
padding-top: 0 !important;
padding-bottom: 0 !important;
border-top-width: 0 !important;
border-bottom-width: 0!important;
height: 0 !important;
}

.dataTables_hiddenSizer tr {
height: 0 !important;
}
[/code]

and then instead of
[code]
nTheadSize = $(o.nTHead).clone()[0];
...
_fnApplyToChildren( zeroOut, anHeadSizers );
...
$(anHeadSizers).height(0);
[/code]

I just applied the style when cloning:
[code]
nTheadSize = $(o.nTHead).clone().addClass("dataTables_hiddenSizer")[0];
[/code]

Not 100% sure, but if it works correctly across all browsers, it should be significantly faster than looping through all the TRs / TDs to set styles.

As a side, note it looked like all the new loops have both anHeadSizers and anHeadToSize passed in as parameters, but each really only needs 1 -- could probably save a little by cutting down to only what's needed.

timtucker

Also noticed that you call getElementsByTagName on nTfootSize twice, one at
[code]
if ( o.nTFoot !== null )
{
_fnApplyToChildren( function(n) {
n.style.width = "";
}, nTfootSize.getElementsByTagName('tr') );
}
[/code]

And later when you define
[code]
anFootSizers = nTfootSize.getElementsByTagName('tr');
[/code]

If you were to move the definition to right after you define nTfootSize, you could save doing the lookup twice.

(Probably also worth testing to see if there's any speed difference in calling it on nTfootSize while it's still detached).

allan

Hi,

Good points. Performance improvements in general are great, but in this area specifically are fantastic!

The reason that I use Javascript to set the styles of the header elements, rather than CSS, is I don't not want to mandate any CSS for using DataTables, and that would be the first that would require it. Obviously you would want to normally use CSS to make the table look half decent, but I didn't want to make it mediatory. I would be interesting to profile how much difference using CSS would make.

Good should the the use of getElementsByTag name - I'll get that sorted out.

Allan

timtucker

I wondered if that would be an issue from a backwards-compatability point.

What about looking into ways to add the class dynamically?

i.e.:
http://yuiblog.com/blog/2007/06/07/style/

It should be possible and might open up some other potential techniques where class manipulation would be faster than working with styles...

allan

Yes that's possible as well. I think it would be worth profiling first, before expending any effort on it. I suspect there are probably other areas which would benefit from performance enhancements more than that.

Allan