From 6b605936f794c0ec83cd5a63a32c4f995f6254d0 Mon Sep 17 00:00:00 2001 From: Allan Jardine Date: Sat, 29 Sep 2012 21:25:41 +0100 Subject: [PATCH] Update: Significant update to how sorting is applied internally in DataTables - there is no difference to how the sort is actually done, with the single exception that the -asc and -desc are not depricated in favour of the -pre method only. - With the introduction of the -pre method in DataTables 1.9, the -asc and -desc sorting functions became more or less redundant since they are simple comparisons (all of the complexity is now in the -pre formatting function). As such the call to the -asc / -desc method is overhead that really isn't needed, and this commit introduces a sort function that doesn't call the -asc / -desc methods, instead just doing the comparison itself. In tests, this relatively simple change leads to a performance improvement of around 15% in all browsers (it also has the side benefit of less operations, so IE8- will be able to sort larger tables before flagging up a slow script warning). - We can't just remove the sorting method which will call -asc / -desc though since not all sorting plug-ins will have a -pre method. Therefore, for backwards compatiblity the old sort function (albeit updated for the changed variables) is retained. The backwards compatibality code adds around 300 bytes to the library, but this is an unaccounced change, so backwards compatiblity must be retained. - The old sort method will be removed in v1.11. The -asc and -desc methods are now fully depricated. - Altered the sorting method to flatten the aaSorting array since the introduction of aDataSort in v1.9 required an extra loop in several locations. The functionality is very useful, but the extra loop can be a bit messy and slightly hit performance, so it is now flattened to be a single array (with object information so it makes sense, rather htan array indexes!). - Altered the order of sorting when building _aSortData since it was looking up the same variable smultiple times which really wasn't needed. This is part of a small incremental changes plan for DataTables! There are still a huge number of things to improve in this area, but this is a nice clean up and a nice 15% sorting performance improvement to get us started :-). --- media/js/jquery.dataTables.js | 200 +++++++++++++---------- media/src/core/core.sort.js | 164 +++++++++++++------ media/src/ext/ext.sorting.js | 31 +--- media/unit_testing/performance/large.php | 27 +-- 4 files changed, 248 insertions(+), 174 deletions(-) diff --git a/media/js/jquery.dataTables.js b/media/js/jquery.dataTables.js index 93fa0f40..1493f441 100644 --- a/media/js/jquery.dataTables.js +++ b/media/js/jquery.dataTables.js @@ -3838,34 +3838,65 @@ * @param {object} oSettings dataTables settings object * @param {bool} bApplyClasses optional - should we apply classes or not * @memberof DataTable#oApi + * @todo This really needs split up! */ function _fnSort ( oSettings, bApplyClasses ) { var i, iLen, j, jLen, k, kLen, sDataType, nTh, - aaSort = [], + aSort = [], aiOrig = [], - oSort = DataTable.ext.oSort, + oExtSort = DataTable.ext.oSort, aoData = oSettings.aoData, aoColumns = oSettings.aoColumns, - oAria = oSettings.oLanguage.oAria; - - /* No sorting required if server-side or no sorting array */ - if ( !oSettings.oFeatures.bServerSide && - (oSettings.aaSorting.length !== 0 || oSettings.aaSortingFixed !== null) ) - { - aaSort = ( oSettings.aaSortingFixed !== null ) ? + oAria = oSettings.oLanguage.oAria, + fnFormatter, aDataSort, data, iCol, sType, oSort, + iFormatters = 0, + aaNestedSort = ( oSettings.aaSortingFixed !== null ) ? oSettings.aaSortingFixed.concat( oSettings.aaSorting ) : oSettings.aaSorting.slice(); - + + /* Flatten the aDataSort inner arrays into a single array, otherwise we have nested + * loops in multiple locations + */ + for ( i=0 ; iy ? 1 : 0; + if ( test !== 0 ) { - return iTest; + return sort.dir === 'asc' ? test : -test; } } - } - - return oSort['numeric-asc']( aiOrig[a], aiOrig[b] ); - } ); + + x = aiOrig[a]; + y = aiOrig[b]; + return xy ? 1 : 0; + } ); + } + else { + // Depreciated - remove in 1.11 (providing a plug-in option) + // Not all sort types have formatting methods, so we have to call their sorting + // methods. + oSettings.aiDisplayMaster.sort( function ( a, b ) { + var + x, y, k, l, test, sort, + len=aSort.length, + dataA = aoData[a]._aSortData, + dataB = aoData[b]._aSortData; + + for ( k=0 ; ky ? 1 : 0; + } ); + } } /* Alter the sorting classes to take account of the changes */ @@ -3977,12 +4041,12 @@ /* In ARIA only the first sorting column can be marked as sorting - no multi-sort option */ if ( aoColumns[i].bSortable ) { - if ( aaSort.length > 0 && aaSort[0][0] == i ) + if ( aSort.length > 0 && aSort[0].col == i ) { - nTh.setAttribute('aria-sort', aaSort[0][1]=="asc" ? "ascending" : "descending" ); + nTh.setAttribute('aria-sort', aSort[0].dir=="asc" ? "ascending" : "descending" ); - var nextSort = (aoColumns[i].asSorting[ aaSort[0][2]+1 ]) ? - aoColumns[i].asSorting[ aaSort[0][2]+1 ] : aoColumns[i].asSorting[0]; + var nextSort = (aoColumns[i].asSorting[ aSort[0].index+1 ]) ? + aoColumns[i].asSorting[ aSort[0].index+1 ] : aoColumns[i].asSorting[0]; nTh.setAttribute('aria-label', sTitle+ (nextSort=="asc" ? oAria.sSortAscending : oAria.sSortDescending) ); } @@ -6221,7 +6285,6 @@ "_fnGetTdNodes": _fnGetTdNodes, "_fnEscapeRegex": _fnEscapeRegex, "_fnDeleteIndex": _fnDeleteIndex, - "_fnReOrderIndex": _fnReOrderIndex, "_fnColumnOrdering": _fnColumnOrdering, "_fnLog": _fnLog, "_fnClearTable": _fnClearTable, @@ -8244,8 +8307,8 @@ /** - * Enable or disable the addition of the classes 'sorting_1', 'sorting_2' and - * 'sorting_3' to the columns which are currently being sorted on. This is + * Enable or disable the addition of the classes `sorting\_1`, `sorting\_2` and + * `sorting\_3` to the columns which are currently being sorted on. This is * presented as a feature switch as it can increase processing time (while * classes are removed and added) so for large data sets you might want to * turn this off. @@ -11640,6 +11703,7 @@ return a.toLowerCase(); }, + // string-asc and -desc are retained only for compatibility with "string-asc": function ( x, y ) { return ((x < y) ? -1 : ((x > y) ? 1 : 0)); @@ -11659,16 +11723,6 @@ return a.replace( /<.*?>/g, "" ).toLowerCase(); }, - "html-asc": function ( x, y ) - { - return ((x < y) ? -1 : ((x > y) ? 1 : 0)); - }, - - "html-desc": function ( x, y ) - { - return ((x < y) ? 1 : ((x > y) ? -1 : 0)); - }, - /* * date sorting @@ -11683,16 +11737,6 @@ } return x; }, - - "date-asc": function ( x, y ) - { - return x - y; - }, - - "date-desc": function ( x, y ) - { - return y - x; - }, /* @@ -11702,16 +11746,6 @@ { return (a=="-" || a==="") ? 0 : a*1; }, - - "numeric-asc": function ( x, y ) - { - return x - y; - }, - - "numeric-desc": function ( x, y ) - { - return y - x; - } } ); diff --git a/media/src/core/core.sort.js b/media/src/core/core.sort.js index c1102a4e..a26f29bf 100644 --- a/media/src/core/core.sort.js +++ b/media/src/core/core.sort.js @@ -3,34 +3,65 @@ * @param {object} oSettings dataTables settings object * @param {bool} bApplyClasses optional - should we apply classes or not * @memberof DataTable#oApi + * @todo This really needs split up! */ function _fnSort ( oSettings, bApplyClasses ) { var i, iLen, j, jLen, k, kLen, sDataType, nTh, - aaSort = [], + aSort = [], aiOrig = [], - oSort = DataTable.ext.oSort, + oExtSort = DataTable.ext.oSort, aoData = oSettings.aoData, aoColumns = oSettings.aoColumns, - oAria = oSettings.oLanguage.oAria; - - /* No sorting required if server-side or no sorting array */ - if ( !oSettings.oFeatures.bServerSide && - (oSettings.aaSorting.length !== 0 || oSettings.aaSortingFixed !== null) ) - { - aaSort = ( oSettings.aaSortingFixed !== null ) ? + oAria = oSettings.oLanguage.oAria, + fnFormatter, aDataSort, data, iCol, sType, oSort, + iFormatters = 0, + aaNestedSort = ( oSettings.aaSortingFixed !== null ) ? oSettings.aaSortingFixed.concat( oSettings.aaSorting ) : oSettings.aaSorting.slice(); - + + /* Flatten the aDataSort inner arrays into a single array, otherwise we have nested + * loops in multiple locations + */ + for ( i=0 ; iy ? 1 : 0; + if ( test !== 0 ) { - return iTest; + return sort.dir === 'asc' ? test : -test; } } - } - - return oSort['numeric-asc']( aiOrig[a], aiOrig[b] ); - } ); + + x = aiOrig[a]; + y = aiOrig[b]; + return xy ? 1 : 0; + } ); + } + else { + // Depreciated - remove in 1.11 (providing a plug-in option) + // Not all sort types have formatting methods, so we have to call their sorting + // methods. + oSettings.aiDisplayMaster.sort( function ( a, b ) { + var + x, y, k, l, test, sort, + len=aSort.length, + dataA = aoData[a]._aSortData, + dataB = aoData[b]._aSortData; + + for ( k=0 ; ky ? 1 : 0; + } ); + } } /* Alter the sorting classes to take account of the changes */ @@ -142,12 +206,12 @@ function _fnSort ( oSettings, bApplyClasses ) /* In ARIA only the first sorting column can be marked as sorting - no multi-sort option */ if ( aoColumns[i].bSortable ) { - if ( aaSort.length > 0 && aaSort[0][0] == i ) + if ( aSort.length > 0 && aSort[0].col == i ) { - nTh.setAttribute('aria-sort', aaSort[0][1]=="asc" ? "ascending" : "descending" ); + nTh.setAttribute('aria-sort', aSort[0].dir=="asc" ? "ascending" : "descending" ); - var nextSort = (aoColumns[i].asSorting[ aaSort[0][2]+1 ]) ? - aoColumns[i].asSorting[ aaSort[0][2]+1 ] : aoColumns[i].asSorting[0]; + var nextSort = (aoColumns[i].asSorting[ aSort[0].index+1 ]) ? + aoColumns[i].asSorting[ aSort[0].index+1 ] : aoColumns[i].asSorting[0]; nTh.setAttribute('aria-label', sTitle+ (nextSort=="asc" ? oAria.sSortAscending : oAria.sSortDescending) ); } diff --git a/media/src/ext/ext.sorting.js b/media/src/ext/ext.sorting.js index 93ab015b..7810a4d5 100644 --- a/media/src/ext/ext.sorting.js +++ b/media/src/ext/ext.sorting.js @@ -11,6 +11,7 @@ $.extend( DataTable.ext.oSort, { return a.toLowerCase(); }, + // string-asc and -desc are retained only for compatibility with "string-asc": function ( x, y ) { return ((x < y) ? -1 : ((x > y) ? 1 : 0)); @@ -30,16 +31,6 @@ $.extend( DataTable.ext.oSort, { return a.replace( /<.*?>/g, "" ).toLowerCase(); }, - "html-asc": function ( x, y ) - { - return ((x < y) ? -1 : ((x > y) ? 1 : 0)); - }, - - "html-desc": function ( x, y ) - { - return ((x < y) ? 1 : ((x > y) ? -1 : 0)); - }, - /* * date sorting @@ -54,16 +45,6 @@ $.extend( DataTable.ext.oSort, { } return x; }, - - "date-asc": function ( x, y ) - { - return x - y; - }, - - "date-desc": function ( x, y ) - { - return y - x; - }, /* @@ -73,14 +54,4 @@ $.extend( DataTable.ext.oSort, { { return (a=="-" || a==="") ? 0 : a*1; }, - - "numeric-asc": function ( x, y ) - { - return x - y; - }, - - "numeric-desc": function ( x, y ) - { - return y - x; - } } ); diff --git a/media/unit_testing/performance/large.php b/media/unit_testing/performance/large.php index c4911528..83819ea8 100644 --- a/media/unit_testing/performance/large.php +++ b/media/unit_testing/performance/large.php @@ -23,25 +23,30 @@