Contribution: Performance improvements

Contribution: Performance improvements

aabricaabric Posts: 3Questions: 0Answers: 0
edited June 2013 in General
Hi,

I would like to share with you the performance improvements that we implemented, so that you can give us your comments on it, and potentially integrate them into the original code.

We were using DataTables 1.9.3 with data sets that contain up to 50 columns, including fixed columns, and the rendering in Internet Explorer was taking more than 20s. So we implemented these changes, in addition to the use of mRender (instead of fnRender) and the performance optimizations of the latest 1.9.4 version. After these changes, the rendering is 2 to 10 times faster, depending on the amount of rows and columns, as well as the data used.

------------------------------------------------------------------------------------------
CHANGE #1: Setting innerHTML in IE is slow. It should not be called for empty cells or cells that don't contain HTML.
------------------------------------------------------------------------------------------

In function _fnCreateTr(), replace this code :
[code]
nTd.innerHTML = (typeof oCol.fnRender === 'function' && (!oCol.bUseRendered || oCol.mData === null)) ?
_fnRender( oSettings, iRow, i ) :
_fnGetCellData( oSettings, iRow, i, 'display' );
[/code]
with this code:
[code]
var cellContent = (typeof oCol.fnRender === 'function' && (!oCol.bUseRendered || oCol.mData === null)) ?
_fnRender(oSettings, iRow, i) :
_fnGetCellData(oSettings, iRow, i, 'display');

if (cellContent && cellContent.length) {
if (typeof oCol.mRender === 'function') {
nTd.innerHTML = cellContent;
} else if (cellContent.indexOf("&") !== -1) {
nTd.innerHTML = cellContent;
} else {
nTd.innerText = cellContent;
}
}
[/code]

------------------------------------------------------------------------------------------
CHANGE #2: Fixed Columns plugin triggers a second grid rendering. Initial grid rendering is not necessary if the fixed columns plugin is doing it.
------------------------------------------------------------------------------------------

In function _fnAjaxUpdateDraw(), replace this code:
[code]
_fnDraw( oSettings );
[/code]
with this code:
[code]
if (!oSettings.isInitialLoadPerformed && oSettings.isFnDrawProcessedByFixedColumnPlugin) {
// _fnDraw will be called by the plugin, so we don't need to call it now.
_fnInitComplete(oSettings);
}
else {
_fnDraw(oSettings);
}
oSettings.isInitialLoadPerformed = true;
[/code]

Under the section "// Map the initialisation options onto the settings object", add the code:
[code]
_fnMap(oSettings, oInit, "isFnDrawProcessedByFixedColumnPlugin");
oSettings.isInitialLoadPerformed = false;
[/code]

Of course, in order for this trick to work, you must initiate your datatable with the new option isFnDrawProcessedByFixedColumnPlugin to true.

In function _fnColCalc() of FixedColumn plugin :
[code]
$('tbody>tr:eq(0)>td, tbody>tr:eq(0)>th', this.s.dt.nTable).each(function (i) {
if (i < that.s.iLeftColumns || that.s.iTableColumns - that.s.iRightColumns <= i) {
iWidth = new Number(columns[i].sWidth.slice(0, -2));

// Inner width is used to assign widths to cells
that.s.aiInnerWidths.push(iWidth);

// Outer width is used to calculate the container
that.s.aiOuterWidths.push(iWidth + 7); // Hardcoded ... not optimal

if (i < that.s.iLeftColumns) {
iLeftWidth += iWidth + 7; // Hardcoded ... not optimal
}
if (that.s.iTableColumns - that.s.iRightColumns <= i) {
iRightWidth += iWidth;
}
}
});
[/code]

Thank you for your comments !

Alex

Replies

  • allanallan Posts: 63,514Questions: 1Answers: 10,472 Site admin
    Thank you for these. I will look at integrating them into DataTables and do a bit more analysis on them.

    Regards,
    Allan
  • allanallan Posts: 63,514Questions: 1Answers: 10,472 Site admin
    I've put together a few tests on JSPerf to check what performance enhancement we might be able to get but using innerText / textContent, and it doesn't look like much (actually in most cases the performances drops).

    I've put three test cases up:

    1. Plain innerHTML to serve as the base line

    2. Detecting if innerHTML should be used through use of indexOf() looking for & and < characters (and using textContent for W3C compliant browsers - textContent is IE specific)

    3. Detecting if innerHTML should be used through a regular expression

    The results show that:

    Webkit - innerHTML is massively faster

    Firefox - textContent is slightly faster

    IE 9 & 10 - innerHTML is slightly faster
    IE 7 & 8 - innerText is slightly faster

    So I don't think I'll include this into DataTables core at the moment, since innerHTML is demonstrability faster in modern browsers, with the exception of Firefox, who's innerHTML implementation is already one of the best.

    Regards,
    Allan
  • aabricaabric Posts: 3Questions: 0Answers: 0
    Hi Allan,

    Thanks for the feedback and having a look at this code.

    As our users have IE7 for 80% of them, the improvement consisting in conditionally replacing innerHTML to innerText did bring something. Also, I suppose the improvement level depends on the actual data you have to display in the grid (the larger the HTML content is, the more time it takes, I guess). But you are right, it is not a massive change in the performance, probably about 10%. What do you think about adding a browser check to enable/disable this performance tweak ?

    However, the first change isn't only the innerHTML conditional change to innerText. It is also about populating the cell when the content is not empty. Again, it depends on your data, and for us, it was very useful. And it is a safe "quick win" to put the cellcontent in an intermediate variable and add this "IF" statement:

    [code]
    if (cellContent && cellContent.length) {
    [/code]

    The second change (FixedColumns) brought us a much bigger performance improvement. Did you have the change to look at it ?

    Thanks, Alex
  • allanallan Posts: 63,514Questions: 1Answers: 10,472 Site admin
    > What do you think about adding a browser check to enable/disable this performance tweak ?

    Given that its for IE7/8 and it would add code weight (not much I admit, but some) to the core library, I'll likely not be implementing this. Indeed, with IE6/7/8 on their way out, DataTables support for those browsers will gradually diminish. The next releases of the plug-ins will not include support (they might work, they might not, but there won't be any work around for old IE bugs), and possibly DataTables from v1.11 (although it only effects scrolling).

    Regarding the empty data - it would need to be a check for `cellContent===''` since an integer etc could be given. That's possible to add, but in my experience it is unusual to have a large number of empty cells in a DataTable. I assume you are ajax loading your data?

    > The second change (FixedColumns) brought us a much bigger performance improvement. Did you have the change to look at it ?

    Not yet - I will do as part of the next update to FixedColumns (currently my attention is all on DataTables :-) ).

    Regards,
    Allan
This discussion has been closed.