Novocaine
Novocaine

Reputation: 4786

Formatting table with jQuery is very slow

I've got a table that has a toggle to show/hide various rows that match certain criteria and I'm using the following JS to achieve what I want (JSFiddle);

function resetRows() {
    var i = 1,
        tds;
    var start = new Date().getTime();
    $('td').removeClass('even odd');

    $.each($('tr'), function (key, index) {
        // loop through each table row skipping the first row which will be the title
        tds = $(this).find($('td'));
        if ($(this).is(':visible') && key > 0) {
            if (i % 2 === 0) {
                tds.addClass('even');
            } else {
                tds.addClass('odd');
            }
            // find the .index class and reset it's value
            $(this).find($('.index')).html(i);
            i++;
        }
    });
    var end = new Date().getTime();
    var time = end - start;
    console.log('time taken: ' + time);
}

$(function () {

    // show/hide failed
    $('#showHideFails').on('click', function () {
        if ($('.failed').eq(0).is(':visible')) {
            $('.failed, .ins').hide();
            $('.shTxt').html('Show all');
            resetRows();
        } else {
            $('.failed, .ins').show();
            $('.shTxt').html('Hide failed');
            resetRows();
        }
    });

});

My problem is that this code is executing very slowly. The js fiddle is relatively quick as my example table only has 15 rows. My actual table has 100 rows, and can be changed to show 1,000 rows.

I've added a console log to test how long it's executing and in the fiddle it's taking ~15ms to hide the rows and ~20-40ms to show the rows again.

On my real table with 100 rows this changes to ~300-450ms to hide and ~500-600ms to show. This figure more than doubles when trying 200 rows (1,200 and 2,00 respectively). When I tried with 1,000 rows my browser nearly crashed.

I tried changing my code so that instead of changing the class names of td's it changed the class names of the tr's as in theory this would mean less processing required for jQuery to handle, however this almost doubled the execution time so I reverted back to td's

Can anyone tell me how to make my code more efficient? It's a little frustrating to get the functionality that I want, only to have it run very slowly.

Upvotes: 0

Views: 577

Answers (2)

Friederike
Friederike

Reputation: 1282

Some code optimization suggestions:

  1. usage of .find()

    tds = $(this).find($('td')); is the same as tds = $(this).find('td')

    $('td') causes jquery to search for all table cells in your whole HTML before doing anything else

    But what you really want is: All cells for the current row only which can be done with $(this).find('td')

  2. Remove "dead" code

    Your row-iteration-function does nothing if you're in the header-row or if the row has already been hidden. At the moment you're searching for cells in that row even if it is hidden.

     $.each($('tr'), function (key, index) {
        if (false == $(this).is(':visible') || 0 == key) {
            // skip hidden rows and header row
            return;
        }
    
        // at this point we know that the current row is visible and not the header row
        tds = $(this).find($('td'));
        ...
     });
    
  3. Reduce jQuery-selectors

    As far as I can see the selectors are the most cost-intensive part here. While that's not a problem for small data it can effect large data like your table with 1000 rows and such. Try to re-use the selector-results where possible by using a variable.

    Replace $(this) with a variable:

    var currentRow = $(this);
    currentRow.is(':visible');
    currentRow.find('td');
    ...
    

These are my thoughts so far. See this jsfiddle for rewritten code: http://jsfiddle.net/5jVdh/3/

Upvotes: 1

A. Wolff
A. Wolff

Reputation: 74420

You should detach your table from DOM when using this kind of elements iteration. You could then show any message to user like a loader during task.

http://jsfiddle.net/ZYsHL/6/

function resetRows($table) {
    var $loader = $('<div id="loader"/>').insertBefore($table),
        $tblContent = $table.detach(),
        i = 1,
        trs = $tblContent.find('tr'),
        tds = $tblContent.find('td').removeClass('even odd');
    var start = new Date().getTime();

    $.each(trs, function (key, index) {
        // loop through each table row skipping the first row which will be the title
        var tdsRow = $(this).find('td');

        if (!$(this).data('hidden') && key > 0) {
            if (i % 2 === 0) {
                tdsRow.addClass('even');
            } else {
                tdsRow.addClass('odd');
            }
            // find the .index class and reset it's value
            $(this).find('.index').html(i);
            i++;
            console.log(i);
        }
    });
    var end = new Date().getTime();
    var time = end - start;
    $loader.replaceWith($table);
    console.log('time taken: ' + time);
}

$(function () {

    // show/hide failed
    $('#showHideFails').on('click', function () {
        if ($('.failed').eq(0).is(':visible')) {
            $('.failed, .ins').data('hidden',true).hide();
            $('.shTxt').html('Show all');
            resetRows($('#table'));
        } else {
            $('.failed, .ins').data('hidden',false).show();
            $('.shTxt').html('Hide failed');
            resetRows($('#table'));
        }
    });

});

Upvotes: 3

Related Questions